You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apex.apache.org by siyuanh <gi...@git.apache.org> on 2015/11/06 01:25:23 UTC
[GitHub] incubator-apex-core pull request: Lazy load type graph
GitHub user siyuanh opened a pull request:
https://github.com/apache/incubator-apex-core/pull/158
Lazy load type graph
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/siyuanh/incubator-apex-core APEX-188
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-apex-core/pull/158.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #158
----
commit c9ee8da653507076d1f06be624479b8f89ce536d
Author: siyuan <hs...@gmail.com>
Date: 2015-11-06T00:20:16Z
APEX-188 #resolve #comment read only class metadata(class signature with default constructor) when construct the type graph and load the detail until it is required for example describe class
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-core pull request: Lazy load type graph
Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on a diff in the pull request:
https://github.com/apache/incubator-apex-core/pull/158#discussion_r44195302
--- Diff: engine/src/main/java/com/datatorrent/stram/webapp/asm/FastClassIndexReader.java ---
@@ -0,0 +1,341 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package com.datatorrent.stram.webapp.asm;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.UnsupportedEncodingException;
+
+import org.apache.xbean.asm5.Opcodes;
+
+/**
+ * Created by siyuan on 10/29/15.
+ *
+ * An improvement for ASM class reader.
+ * ASM class reader reads the whole class into structured data like ClassNode MethodNode
+ * But to build a type graph we only need to know class name, parent class name, interfaces name and
+ * whether it has a public non-arg default constructor (instantiable)
+ * This class skip most parts that are not necessary and parse only the information above
+ *
+ * And also it use shared buffer to load classes which makes it faster in our performance test
+ *
+ * Overall it is 8-9x faster than ASM class reader
+ *
+ * Keep in mind it is NOT thread safe
+ *
+ */
+public class FastClassIndexReader
+{
+
+ /**
+ * The type of CONSTANT_Class constant pool items.
+ */
+ static final int CLASS = 7;
+
+ /**
+ * The type of CONSTANT_Fieldref constant pool items.
+ */
+ static final int FIELD = 9;
+
+ /**
+ * The type of CONSTANT_Methodref constant pool items.
+ */
+ static final int METH = 10;
+
+ /**
+ * The type of CONSTANT_InterfaceMethodref constant pool items.
+ */
+ static final int IMETH = 11;
+
+ /**
+ * The type of CONSTANT_String constant pool items.
+ */
+ static final int STR = 8;
+
+ /**
+ * The type of CONSTANT_Integer constant pool items.
+ */
+ static final int INT = 3;
+
+ /**
+ * The type of CONSTANT_Float constant pool items.
+ */
+ static final int FLOAT = 4;
+
+ /**
+ * The type of CONSTANT_Long constant pool items.
+ */
+ static final int LONG = 5;
+
+ /**
+ * The type of CONSTANT_Double constant pool items.
+ */
+ static final int DOUBLE = 6;
+
+ /**
+ * The type of CONSTANT_NameAndType constant pool items.
+ */
+ static final int NAME_TYPE = 12;
+
+ /**
+ * The type of CONSTANT_Utf8 constant pool items.
+ */
+ static final int UTF8 = 1;
+
+ /**
+ * The type of CONSTANT_MethodType constant pool items.
+ */
+ static final int MTYPE = 16;
+
+ /**
+ * The type of CONSTANT_MethodHandle constant pool items.
+ */
+ static final int HANDLE = 15;
+
+ /**
+ * The type of CONSTANT_InvokeDynamic constant pool items.
+ */
+ static final int INDY = 18;
+
+ // shared buffer to hold the content of the file
+ private static byte[] b = new byte[64 * 1024];
+
+ private static int bSize = 0;
+
+ private int[] items;
+
+ private int header;
+
+ private String name;
+
+ private String superName;
+
+ private String[] interfaces;
+
+ // use this for byte array comparison which is faster than string comparison
+ public static final byte[] DEFAULT_CONSTRUCTOR_NAME = new byte[]{0, 6, '<', 'i', 'n', 'i', 't', '>'};
+
+ public static final byte[] DEFAULT_CONSTRUCTOR_DESC = new byte[]{0, 3, '(', ')', 'V'};
+
+ private boolean isInstantiable = false;
+
+ public FastClassIndexReader(final InputStream is) throws IOException
+ {
+ readIntoBuffer(is);
+
+ readConstantPool();
+
+ readIndex();
+ }
+
+ /**
+ * Read class file content into shared buffer from input stream
+ * Stream won't be closed
+ * @param is
+ * @throws IOException
+ */
+ private void readIntoBuffer(InputStream is) throws IOException
+ {
+ if (is == null) {
+ throw new IOException("Class not found");
+ }
+ bSize = 0;
+ while (true) {
+ int n = is.read(b, bSize, b.length - bSize);
+ if (n == -1) {
+ break;
+ }
+ bSize += n;
+ if (bSize >= b.length) {
+ byte[] c = new byte[b.length << 2];
+ System.arraycopy(b, 0, c, 0, b.length);
+ b = c;
+ }
+ }
+ }
+
+ /**
+ * read and index the constant pool section for getting class metadata later
+ */
+ private void readConstantPool()
+ {
+ // checks the class version
+ if (readShort(6) > Opcodes.V1_8) {
+ throw new IllegalArgumentException();
+ }
+ // parses the constant pool
+ items = new int[readUnsignedShort(8)];
+ int n = items.length;
+ int index = 10;
+ for (int i = 1; i < n; ++i) {
+ items[i] = index + 1;
+ int size;
+ switch (b[index]) {
+ case FIELD:
+ case METH:
+ case IMETH:
+ case INT:
+ case FLOAT:
+ case NAME_TYPE:
+ case INDY:
+ size = 5;
+ break;
+ case LONG:
+ case DOUBLE:
+ size = 9;
+ ++i;
+ break;
+ case UTF8:
+ size = 3 + readUnsignedShort(index + 1);
+ break;
+ case HANDLE:
+ size = 4;
+ break;
+ // case ClassWriter.CLASS:
+ // case ClassWriter.STR:
+ // case ClassWriter.MTYPE
+ default:
+ size = 3;
+ break;
+ }
+ index += size;
+ }
+ // the class header information starts just after the constant pool
+ header = index;
+ }
+
+ /**
+ * read class metadata, class name, parent name, interfaces name, is instantiable(has public non-arg constructor)
+ * or not
+ * @throws UnsupportedEncodingException
+ */
+ private void readIndex() throws UnsupportedEncodingException
+ {
+ // reads the class declaration
+ int u = header;
+
+ int access = readUnsignedShort(u);
+ isInstantiable = ASMUtil.isPublic(access) && !ASMUtil.isAbstract(access);
+
+ name = readClass(u + 2);
+ superName = readClass(u + 4);
+ interfaces = new String[readUnsignedShort(u + 6)];
+ u += 8;
+ for (int i = 0; i < interfaces.length; ++i) {
+ interfaces[i] = readClass(u);
+ u += 2;
+ }
+
+ if (!isInstantiable) {
+ return;
+ }
+
+ // reads the constructor
+
+ // skip fields
+ for (int i = readUnsignedShort(u); i > 0; --i) {
+ for (int j = readUnsignedShort(u + 8); j > 0; --j) {
+ u += 6 + readInt(u + 12);
+ }
+ u += 8;
+ }
+ u += 2;
+ for (int i = readUnsignedShort(u); i > 0; --i) {
+ if (isDefaultConstructor(u + 2)) {
+ return;
+ }
+ for (int j = readUnsignedShort(u + 8); j > 0; --j) {
+ u += 6 + readInt(u + 12);
+ }
+ u += 8;
+ }
+ isInstantiable = false;
+ }
+
+ private boolean isDefaultConstructor(int methodIndex) throws UnsupportedEncodingException
+ {
+ return arrayContains(b, items[readUnsignedShort(methodIndex + 2)], DEFAULT_CONSTRUCTOR_NAME)
+ && arrayContains(b, items[readUnsignedShort(methodIndex + 4)], DEFAULT_CONSTRUCTOR_DESC)
+ && ASMUtil.isPublic(readUnsignedShort(methodIndex));
+ }
+
+ private boolean arrayContains(byte[] bb, int i, byte[] subArray)
+ {
+ for (int l = 0; l < subArray.length; l++) {
+ if (bb[i + l] != subArray[l]) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ public int readUnsignedShort(final int index)
--- End diff --
public function but has no documentation
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-core pull request: Lazy load type graph
Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/158#issuecomment-154561782
Are the line lengths less than 120? Since you are fixing existing style in these classes maybe you should do this as well
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-core pull request: Lazy load type graph
Posted by gauravgopi123 <gi...@git.apache.org>.
Github user gauravgopi123 commented on a diff in the pull request:
https://github.com/apache/incubator-apex-core/pull/158#discussion_r44212123
--- Diff: engine/src/test/java/com/datatorrent/stram/webapp/OperatorDiscoveryTest.java ---
@@ -1138,5 +1140,7 @@ public void testMethodType()
Assert.assertEquals("@description", OperatorDiscoverer.MethodTagType.DESCRIPTION, OperatorDiscoverer.MethodTagType.from("@description"));
Assert.assertEquals("@random", null, OperatorDiscoverer.MethodTagType.from("@random"));
}
+
--- End diff --
extra line
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-core pull request: Lazy load type graph
Posted by siyuanh <gi...@git.apache.org>.
Github user siyuanh commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/158#issuecomment-155860037
@ishark I have updated the pull request, please take a look
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-core pull request: Lazy load type graph
Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on a diff in the pull request:
https://github.com/apache/incubator-apex-core/pull/158#discussion_r44193760
--- Diff: engine/src/main/java/com/datatorrent/stram/webapp/asm/FastClassIndexReader.java ---
@@ -0,0 +1,341 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package com.datatorrent.stram.webapp.asm;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.UnsupportedEncodingException;
+
+import org.apache.xbean.asm5.Opcodes;
+
+/**
+ * Created by siyuan on 10/29/15.
--- End diff --
Author name in source file?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-core pull request: Lazy load type graph
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-apex-core/pull/158
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-core pull request: Lazy load type graph
Posted by siyuanh <gi...@git.apache.org>.
Github user siyuanh commented on a diff in the pull request:
https://github.com/apache/incubator-apex-core/pull/158#discussion_r44559799
--- Diff: engine/src/main/java/com/datatorrent/stram/webapp/TypeGraph.java ---
@@ -602,6 +558,14 @@ public void setJarName(String jarName)
public CompactClassNode getClassNode()
{
+ if (classNode == null) {
--- End diff --
Good catch, this method should be synchronized
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-core pull request: Lazy load type graph
Posted by ishark <gi...@git.apache.org>.
Github user ishark commented on a diff in the pull request:
https://github.com/apache/incubator-apex-core/pull/158#discussion_r44220893
--- Diff: engine/src/main/java/com/datatorrent/stram/webapp/TypeGraph.java ---
@@ -602,6 +558,14 @@ public void setJarName(String jarName)
public CompactClassNode getClassNode()
{
+ if (classNode == null) {
--- End diff --
Do we need double-checked locking here? Very slim chance, but is it likely that describeOperator gets called simultaneously through dtcli?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
Re: [GitHub] incubator-apex-core pull request: Lazy load type graph
Posted by Thomas Weise <th...@datatorrent.com>.
You trigger Travis by close/reopen of the PR. Jenkins is not around to help
:-)
--
sent from mobile
On Nov 6, 2015 11:25 AM, "siyuanh" <gi...@git.apache.org> wrote:
> Github user siyuanh commented on the pull request:
>
>
> https://github.com/apache/incubator-apex-core/pull/158#issuecomment-154508703
>
> Jenkins test this please
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---
>
[GitHub] incubator-apex-core pull request: Lazy load type graph
Posted by siyuanh <gi...@git.apache.org>.
Github user siyuanh commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/158#issuecomment-154508703
Jenkins test this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-core pull request: Lazy load type graph
Posted by ishark <gi...@git.apache.org>.
Github user ishark commented on a diff in the pull request:
https://github.com/apache/incubator-apex-core/pull/158#discussion_r44220617
--- Diff: engine/src/test/java/com/datatorrent/stram/webapp/OperatorDiscoveryTest.java ---
@@ -24,6 +24,7 @@
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.File;
+import java.io.IOException;
--- End diff --
No new change in this file.. Unused imports?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-apex-core pull request: Lazy load type graph
Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on a diff in the pull request:
https://github.com/apache/incubator-apex-core/pull/158#discussion_r44195160
--- Diff: engine/src/test/java/com/datatorrent/stram/webapp/asm/FastClassIndexReaderTest.java ---
@@ -0,0 +1,112 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package com.datatorrent.stram.webapp.asm;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.jar.JarEntry;
+import java.util.jar.JarFile;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.xbean.asm5.ClassReader;
+
+/**
+ * Created by siyuan on 10/30/15.
--- End diff --
author name ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---