Skip to content
Browse files

Partial fix for too-big methods (JRUBY-2621 and JRUBY-4757): chunk 10…

…0%-literal arrays and hashes.
  • Loading branch information...
1 parent e69ee73 commit 829cd0a425dd3fafd0da7af01f3c86b08a5fe81d @headius headius committed
View
75 src/org/jruby/compiler/ASTCompiler.java
@@ -615,14 +615,19 @@ public void compileArray(Node node, BodyCompiler context, boolean expr) {
if (doit) {
ArrayCallback callback = new ArrayCallback() {
+ public void nextValue(BodyCompiler context, Object sourceArray, int index) {
+ Node node = (Node) ((Object[]) sourceArray)[index];
+ compile(node, context, true);
+ }
+ };
- public void nextValue(BodyCompiler context, Object sourceArray, int index) {
- Node node = (Node) ((Object[]) sourceArray)[index];
- compile(node, context, true);
- }
- };
+ List<Node> childNodes = arrayNode.childNodes();
- context.createNewArray(arrayNode.childNodes().toArray(), callback, arrayNode.isLightweight());
+ if (isListAllLiterals(arrayNode)) {
+ context.createNewLiteralArray(childNodes.toArray(), callback, arrayNode.isLightweight());
+ } else {
+ context.createNewArray(childNodes.toArray(), callback, arrayNode.isLightweight());
+ }
if (popit) context.consumeCurrentValue();
} else {
@@ -633,6 +638,42 @@ public void nextValue(BodyCompiler context, Object sourceArray, int index) {
}
}
+ private boolean isListAllLiterals(ListNode listNode) {
+ for (int i = 0; i < listNode.size(); i++) {
+ switch (listNode.get(i).getNodeType()) {
+ case STRNODE:
+ case FLOATNODE:
+ case FIXNUMNODE:
+ case BIGNUMNODE:
+ case REGEXPNODE:
+ case ZARRAYNODE:
+ case SYMBOLNODE:
+ case NILNODE:
+ // simple literals, continue
+ continue;
+ case ARRAYNODE:
+ // scan contained array
+ if (isListAllLiterals((ArrayNode)listNode.get(i))) {
+ continue;
+ } else {
+ return false;
+ }
+ case HASHNODE:
+ // scan contained hash
+ if (isListAllLiterals(((HashNode)listNode.get(i)).getListNode())) {
+ continue;
+ } else {
+ return false;
+ }
+ default:
+ return false;
+ }
+ }
+
+ // all good!
+ return true;
+ }
+
public void compileArgsCat(Node node, BodyCompiler context, boolean expr) {
ArgsCatNode argsCatNode = (ArgsCatNode) node;
@@ -2465,17 +2506,21 @@ protected void compileHashCommon(HashNode hashNode, BodyCompiler context, boolea
}
ArrayCallback hashCallback = new ArrayCallback() {
+ public void nextValue(BodyCompiler context, Object sourceArray,
+ int index) {
+ ListNode listNode = (ListNode) sourceArray;
+ int keyIndex = index * 2;
+ compile(listNode.get(keyIndex), context, true);
+ compile(listNode.get(keyIndex + 1), context, true);
+ }
+ };
- public void nextValue(BodyCompiler context, Object sourceArray,
- int index) {
- ListNode listNode = (ListNode) sourceArray;
- int keyIndex = index * 2;
- compile(listNode.get(keyIndex), context, true);
- compile(listNode.get(keyIndex + 1), context, true);
- }
- };
+ if (isListAllLiterals(hashNode.getListNode())) {
+ context.createNewLiteralHash(hashNode.getListNode(), hashCallback, hashNode.getListNode().size() / 2);
+ } else {
+ context.createNewHash(hashNode.getListNode(), hashCallback, hashNode.getListNode().size() / 2);
+ }
- createNewHash(context, hashNode, hashCallback);
if (popit) context.consumeCurrentValue();
} else {
for (Node nextNode : hashNode.getListNode().childNodes()) {
View
33 src/org/jruby/compiler/BodyCompiler.java
@@ -159,13 +159,27 @@
public void createNewArray(boolean lightweight);
/**
- * Given an aggregated set of objects (likely created through a call to createObjectArray)
- * create a Ruby array object. This version accepts an array of objects
- * to feed to an ArrayCallback to construct the elements of the array.
+ * Construct a Ruby array given an array of objects to feed to an ArrayCallback
+ * to construct the elements of the array.
+ *
+ * @param sourceArray The objects that will be used to construct elements
+ * @param callback The callback to which to pass the objects
+ * @param lightweight Whether the array should be lightweight
*/
public void createNewArray(Object[] sourceArray, ArrayCallback callback, boolean lightweight);
/**
+ * Construct a Ruby array given an array of objects to feed to an ArrayCallback
+ * to construct the elements of the array. All the elements are guaranteed
+ * to be literals, so the contents can safely be chunked if too large.
+ *
+ * @param sourceArray The objects that will be used to construct elements
+ * @param callback The callback to which to pass the objects
+ * @param lightweight Whether the array should be lightweight
+ */
+ public void createNewLiteralArray(Object[] sourceArray, ArrayCallback callback, boolean lightweight);
+
+ /**
* Create an empty Ruby array
*/
public void createEmptyArray();
@@ -186,6 +200,19 @@
* @param keyCount the total count of key-value pairs to be constructed from the elements collection.
*/
public void createNewHash(Object elements, ArrayCallback callback, int keyCount);
+
+ /**
+ * Create a new hash by calling back to the specified ArrayCallback. It is expected that the keyCount
+ * will be the actual count of key/value pairs, and the caller will handle passing an appropriate elements
+ * collection in and dealing with the sequential indices passed to the callback. This version expects
+ * that all elements will be literals, and will break up the hash construction if it is too large.
+ *
+ * @param elements An object holding the elements from which to create the Hash.
+ * @param callback An ArrayCallback implementation to which the elements array and iteration counts
+ * are passed in sequence.
+ * @param keyCount the total count of key-value pairs to be constructed from the elements collection.
+ */
+ public void createNewLiteralHash(Object elements, ArrayCallback callback, int keyCount);
/**
* @see createNewHash
View
181 src/org/jruby/compiler/impl/BaseBodyCompiler.java
@@ -496,6 +496,10 @@ public void createNewArray(Object[] sourceArray, ArrayCallback callback, boolean
buildRubyArray(sourceArray, callback, lightweight);
}
+ public void createNewLiteralArray(Object[] sourceArray, ArrayCallback callback, boolean lightweight) {
+ buildRubyLiteralArray(sourceArray, callback, lightweight);
+ }
+
public void createEmptyArray() {
loadRuntime();
@@ -574,6 +578,98 @@ private void buildRubyArray(Object[] sourceArray, ArrayCallback callback, boolea
}
}
+ private void buildRubyLiteralArray(Object[] sourceArray, ArrayCallback callback, boolean light) {
+ if (sourceArray.length < 100) {
+ // don't chunk arrays smaller than 100 elements
+ loadRuntime();
+ buildRubyArray(sourceArray, callback, light);
+ } else {
+ // populate the array in a separate series of methods
+ SkinnyMethodAdapter oldMethod = method;
+
+ // prepare the first builder in the chain
+ String methodName = "array_builder_" + script.getAndIncrementMethodIndex() + "";
+ method = new SkinnyMethodAdapter(
+ script.getClassVisitor().visitMethod(
+ ACC_PRIVATE | ACC_SYNTHETIC | ACC_STATIC,
+ methodName,
+ sig(IRubyObject[].class, "L" + script.getClassname() + ";", ThreadContext.class, IRubyObject[].class),
+ null,
+ null));
+ method.start();
+
+ method.aload(1);
+ method.invokevirtual(p(ThreadContext.class), "getRuntime", sig(Ruby.class));
+ method.astore(getRuntimeIndex());
+
+ method.aload(getRuntimeIndex());
+ method.invokevirtual(p(Ruby.class), "getNil", sig(IRubyObject.class));
+ method.astore(getNilIndex());
+
+ for (int i = 0; i < sourceArray.length; i++) {
+ // for every hundred elements, chain to the next call
+ if ((i + 1) % 100 == 0) {
+ String nextName = "array_builder_" + script.getAndIncrementMethodIndex() + "";
+
+ method.aloadMany(0, 1, 2);
+ method.invokestatic(script.getClassname(), nextName,
+ sig(IRubyObject[].class, "L" + script.getClassname() + ";", ThreadContext.class, IRubyObject[].class));
+ method.areturn();
+ method.end();
+
+ method = new SkinnyMethodAdapter(
+ script.getClassVisitor().visitMethod(
+ ACC_PRIVATE | ACC_SYNTHETIC | ACC_STATIC,
+ nextName,
+ sig(IRubyObject[].class, "L" + script.getClassname() + ";", ThreadContext.class, IRubyObject[].class),
+ null,
+ null));
+ method.start();
+
+ method.aload(1);
+ method.invokevirtual(p(ThreadContext.class), "getRuntime", sig(Ruby.class));
+ method.astore(getRuntimeIndex());
+
+ method.aload(getRuntimeIndex());
+ method.invokevirtual(p(Ruby.class), "getNil", sig(IRubyObject.class));
+ method.astore(getNilIndex());
+ }
+
+ method.aload(2);
+ method.pushInt(i);
+
+ callback.nextValue(this, sourceArray, i);
+
+ method.arraystore();
+ }
+
+ // close out the last method in the chain
+ method.aload(2);
+ method.areturn();
+ method.end();
+
+ // restore original method, prepare runtime and array, and invoke the chain
+ method = oldMethod;
+
+ loadRuntime(); // for newArray* call below
+
+ // chain invoke
+ method.aload(StandardASMCompiler.THIS);
+ method.aload(StandardASMCompiler.THREADCONTEXT_INDEX);
+ method.pushInt(sourceArray.length);
+ method.anewarray(p(IRubyObject.class));
+ method.invokestatic(script.getClassname(), methodName,
+ sig(IRubyObject[].class, "L" + script.getClassname() + ";", ThreadContext.class, IRubyObject[].class));
+
+ // array construct
+ if (light) {
+ method.invokestatic(p(RubyArray.class), "newArrayNoCopyLight", sig(RubyArray.class, Ruby.class, IRubyObject[].class));
+ } else {
+ method.invokestatic(p(RubyArray.class), "newArrayNoCopy", sig(RubyArray.class, Ruby.class, IRubyObject[].class));
+ }
+ }
+ }
+
public void createEmptyHash() {
loadRuntime();
@@ -583,6 +679,10 @@ public void createEmptyHash() {
public void createNewHash(Object elements, ArrayCallback callback, int keyCount) {
createNewHashCommon(elements, callback, keyCount, "constructHash", "fastASetCheckString");
}
+
+ public void createNewLiteralHash(Object elements, ArrayCallback callback, int keyCount) {
+ createNewLiteralHashCommon(elements, callback, keyCount, "constructHash", "fastASetCheckString");
+ }
public void createNewHash19(Object elements, ArrayCallback callback, int keyCount) {
createNewHashCommon(elements, callback, keyCount, "constructHash19", "fastASetCheckString19");
@@ -608,6 +708,87 @@ private void createNewHashCommon(Object elements, ArrayCallback callback, int ke
}
}
+ private void createNewLiteralHashCommon(Object elements, ArrayCallback callback, int keyCount,
+ String constructorName, String methodName) {
+ if (keyCount < 50) {
+ // small hash, use standard construction
+ createNewHashCommon(elements, callback, keyCount, constructorName, methodName);
+ } else {
+ // populate the hash in a separate series of methods
+ SkinnyMethodAdapter oldMethod = method;
+
+ // prepare the first builder in the chain
+ String builderMethod = "hash_builder_" + script.getAndIncrementMethodIndex() + "";
+ method = new SkinnyMethodAdapter(
+ script.getClassVisitor().visitMethod(
+ ACC_PRIVATE | ACC_SYNTHETIC | ACC_STATIC,
+ builderMethod,
+ sig(RubyHash.class, "L" + script.getClassname() + ";", ThreadContext.class, RubyHash.class),
+ null,
+ null));
+ method.start();
+
+ method.aload(1);
+ method.invokevirtual(p(ThreadContext.class), "getRuntime", sig(Ruby.class));
+ method.astore(getRuntimeIndex());
+
+ method.aload(getRuntimeIndex());
+ method.invokevirtual(p(Ruby.class), "getNil", sig(IRubyObject.class));
+ method.astore(getNilIndex());
+
+ for (int i = 0; i < keyCount; i++) {
+ // for every hundred keys, chain to the next call
+ if ((i + 1) % 100 == 0) {
+ String nextName = "hash_builder_" + script.getAndIncrementMethodIndex() + "";
+
+ method.aloadMany(0, 1, 2);
+ method.invokestatic(script.getClassname(), nextName,
+ sig(RubyHash.class, "L" + script.getClassname() + ";", ThreadContext.class, RubyHash.class));
+ method.areturn();
+ method.end();
+
+ method = new SkinnyMethodAdapter(
+ script.getClassVisitor().visitMethod(
+ ACC_PRIVATE | ACC_SYNTHETIC | ACC_STATIC,
+ nextName,
+ sig(RubyHash.class, "L" + script.getClassname() + ";", ThreadContext.class, RubyHash.class),
+ null,
+ null));
+ method.start();
+
+ method.aload(1);
+ method.invokevirtual(p(ThreadContext.class), "getRuntime", sig(Ruby.class));
+ method.astore(getRuntimeIndex());
+
+ method.aload(getRuntimeIndex());
+ method.invokevirtual(p(Ruby.class), "getNil", sig(IRubyObject.class));
+ method.astore(getNilIndex());
+ }
+
+ method.aload(2);
+ loadRuntime();
+ callback.nextValue(this, elements, i);
+ method.invokevirtual(p(RubyHash.class), methodName, sig(void.class, params(Ruby.class, IRubyObject.class, IRubyObject.class)));
+ }
+
+ // close out the last method in the chain
+ method.aload(2);
+ method.areturn();
+ method.end();
+
+ // restore original method
+ method = oldMethod;
+
+ // chain invoke
+ method.aload(StandardASMCompiler.THIS);
+ method.aload(StandardASMCompiler.THREADCONTEXT_INDEX);
+ loadRuntime();
+ method.invokestatic(p(RubyHash.class), "newHash", sig(RubyHash.class, Ruby.class));
+ method.invokestatic(script.getClassname(), builderMethod,
+ sig(RubyHash.class, "L" + script.getClassname() + ";", ThreadContext.class, RubyHash.class));
+ }
+ }
+
public void createNewRange(CompilerCallback beginEndCallback, boolean isExclusive) {
loadRuntime();
loadThreadContext();
View
39 test/testCompiler.rb
@@ -8,6 +8,26 @@
Block = org.jruby.runtime.Block
IRubyObject = org.jruby.runtime.builtin.IRubyObject
+class CompilerTestUtil
+ def self.compile_to_class(src)
+ next_src_id = next_id
+ node = JRuby.parse(src, "testCompiler#{next_src_id}", false)
+ filename = node.position.file
+ classname = filename.sub("/", ".").sub("\\", ".").sub(".rb", "")
+ inspector = ASTInspector.new
+ inspector.inspect(node)
+ context = StandardASMCompiler.new(classname, filename)
+ compiler = ASTCompiler.new
+ compiler.compileRoot(node, context, inspector)
+
+ context.loadClass(JRuby.runtime.getJRubyClassLoader)
+ end
+
+ def self.next_id
+ @foo ? (@foo += 1) : (@foo = 0)
+ end
+end
+
def silence_warnings
verb = $VERBOSE
$VERBOSE = nil
@@ -17,16 +37,7 @@ def silence_warnings
end
def compile_to_class(src)
- node = JRuby.parse(src, "testCompiler#{src.object_id}", false)
- filename = node.position.file
- classname = filename.sub("/", ".").sub("\\", ".").sub(".rb", "")
- inspector = ASTInspector.new
- inspector.inspect(node)
- context = StandardASMCompiler.new(classname, filename)
- compiler = ASTCompiler.new
- compiler.compileRoot(node, context, inspector)
-
- context.loadClass(JRuby.runtime.getJRubyClassLoader)
+ CompilerTestUtil.compile_to_class(src)
end
def compile_and_run(src)
@@ -528,3 +539,11 @@ def self_check; if self; true; else; false; end; end
[NilClass, FalseClass].each {|c| c.__send__ :include, SelfCheck}
test_equal false, nil.self_check
test_equal false, false.self_check
+
+# JRUBY-4757 and JRUBY-2621: can't compile large array/hash
+large_array = (1..10000).to_a.inspect
+large_hash = large_array
+large_hash.gsub!('[', '{')
+large_hash.gsub!(']', '}')
+test_equal(eval(large_array), compile_and_run(large_array))
+test_equal(eval(large_hash), compile_and_run(large_hash))

0 comments on commit 829cd0a

Please sign in to comment.
Something went wrong with that request. Please try again.