From bfb53c8a1ba2dd68ef51583e9c80975127f2e722 Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Wed, 4 Dec 2024 17:02:11 -0600 Subject: [PATCH] Various optimizations for array creation * Use more newRawArray where appropriate * Use newEmptyArray for more arrays intended to be empty * Provide "each" logic for global names * More efficient iteration of Java methods in JavaLang Class ext --- core/src/main/java/org/jruby/RubyArray.java | 33 +++++++----- core/src/main/java/org/jruby/RubyGlobal.java | 3 +- core/src/main/java/org/jruby/RubyKernel.java | 10 ++-- core/src/main/java/org/jruby/RubyModule.java | 4 +- .../org/jruby/ext/ffi/AbstractMemory.java | 5 +- .../jruby/ext/ffi/jffi/VariadicInvoker.java | 7 ++- .../org/jruby/ext/socket/RubyTCPSocket.java | 2 +- .../org/jruby/ext/socket/SocketUtils.java | 4 +- .../internal/runtime/GlobalVariables.java | 10 ++++ .../ExitableInterpreterEngine.java | 3 +- .../org/jruby/javasupport/ext/JavaLang.java | 52 ++++++++++++++----- .../main/java/org/jruby/parser/Parser.java | 3 +- .../java/org/jruby/util/io/PopenExecutor.java | 22 ++++---- 13 files changed, 102 insertions(+), 56 deletions(-) diff --git a/core/src/main/java/org/jruby/RubyArray.java b/core/src/main/java/org/jruby/RubyArray.java index 54653dd2a2a..bb18da35b16 100644 --- a/core/src/main/java/org/jruby/RubyArray.java +++ b/core/src/main/java/org/jruby/RubyArray.java @@ -1035,9 +1035,9 @@ public IRubyObject fetch_values(ThreadContext context, IRubyObject[] args, Block // FIXME: lookup the bounds part of this in error message?? if (index >= arraySize) { if (!block.isGiven()) throw context.runtime.newIndexError("index " + index + " outside of array bounds: 0...0"); - result.append(block.yield(context, asFixnum(context, index))); + result.append(context, block.yield(context, asFixnum(context, index))); } else { - result.append(eltOk(index)); + result.append(context, eltOk(index)); } } @@ -1357,7 +1357,7 @@ public IRubyObject values_at(ThreadContext context, IRubyObject[] args) { for (int i = 0; i < args.length; i++) { final IRubyObject arg = args[i]; if (arg instanceof RubyFixnum fix) { - result.append(entry(fix.value)); + result.append(context, entry(fix.value)); continue; } @@ -1370,11 +1370,11 @@ public IRubyObject values_at(ThreadContext context, IRubyObject[] args) { final int beg = begLen[0]; final int len = begLen[1]; for (int j = 0; j < len; j++) { - result.append( entry(j + beg) ); + result.append(context, entry(j + beg)); } continue; } - result.append(entry(numericToLong(context, arg))); + result.append(context, entry(numericToLong(context, arg))); } return result.finishRawArray(context); @@ -1463,7 +1463,7 @@ public IRubyObject subseq_step(ThreadContext context, RubyArithmeticSequence arg RubyArray result = result1; for(long i = 0; i < len; ++i) { - result.append(eltOk(j)); + result.append(context, eltOk(j)); j = j + step; } @@ -1596,7 +1596,7 @@ public RubyArray push(IRubyObject[] items) { public RubyArray push(ThreadContext context, IRubyObject[] items) { if (items.length == 0) modifyCheck(context); for (IRubyObject item : items) { - append(item); + append(context, item); } return this; } @@ -2919,7 +2919,8 @@ public RubyArray collectArray(ThreadContext context, Block block) { for (int i = 0; i < realLength; i++) { // Do not coarsen the "safe" check, since it will misinterpret AIOOBE from the yield (see JRUBY-5434) - ary.append(block.yieldNonArray(context, eltOk(i), null)); // arr[i] = ... + // arr[i] = ... + ary.append(context, block.yieldNonArray(context, eltOk(i), null)); } return ary.finishRawArray(context); @@ -3020,7 +3021,7 @@ public IRubyObject selectCommon(ThreadContext context, Block block) { for (int i = 0; i < realLength; i++) { // Do not coarsen the "safe" check, since it will misinterpret AIOOBE from the yield (see JRUBY-5434) IRubyObject value = eltOk(i); - if (block.yield(context, value).isTrue()) result.append(value); + if (block.yield(context, value).isTrue()) result.append(context, value); } return result.finishRawArray(context); @@ -3551,12 +3552,12 @@ protected boolean flatten(ThreadContext context, final int level, final RubyArra while (i < ary.realLength) { IRubyObject elt = ary.eltOk(i++); if (level >= 0 && stack.size() / 2 >= level) { - result.append(elt); + result.append(context, elt); continue; } tmp = TypeConverter.checkArrayType(context, elt); if (tmp.isNil()) { - result.append(elt); + result.append(context, elt); } else { // nested array element if (memo != null) { if (memo.get(tmp) != null) throw argumentError(context, "tried to flatten recursive array"); @@ -3926,7 +3927,7 @@ public IRubyObject difference(ThreadContext context, IRubyObject[] args) { if (arrays[j].includesByEql(context, elt)) break; } } - if (j == args.length) diff.append(elt); + if (j == args.length) diff.append(context, elt); } return diff; @@ -4094,7 +4095,7 @@ private void unionInternal(ThreadContext context, RubyArray... args) { for (int j = 0; j < args[i].realLength; j++) { IRubyObject elt = args[i].elt(j); if (includesByEql(context, elt)) continue; - append(elt); + append(context, elt); } } } @@ -5824,7 +5825,11 @@ public T toJava(Class target) { } public boolean add(Object element) { - append(JavaUtil.convertJavaToUsableRubyObject(metaClass.runtime, element)); + return add(getRuntime().getCurrentContext(), element); + } + + public boolean add(ThreadContext context, Object element) { + append(context, JavaUtil.convertJavaToUsableRubyObject(metaClass.runtime, element)); return true; } diff --git a/core/src/main/java/org/jruby/RubyGlobal.java b/core/src/main/java/org/jruby/RubyGlobal.java index dade286b0e5..c899be9f83b 100644 --- a/core/src/main/java/org/jruby/RubyGlobal.java +++ b/core/src/main/java/org/jruby/RubyGlobal.java @@ -103,12 +103,13 @@ public class RubyGlobal { public static void initARGV(Ruby runtime) { var context = runtime.getCurrentContext(); // define ARGV and $* for this runtime - var argvArray = newArray(context); String[] argv = runtime.getInstanceConfig().getArgv(); + var argvArray = newRawArray(context, argv.length); for (String arg : argv) { argvArray.append(context, RubyString.newInternalFromJavaExternal(runtime, arg)); } + argvArray.finishRawArray(context); if (runtime.getObject().getConstantNoConstMissing("ARGV") != null) { ((RubyArray)runtime.getObject().getConstant("ARGV")).replace(context, argvArray); diff --git a/core/src/main/java/org/jruby/RubyKernel.java b/core/src/main/java/org/jruby/RubyKernel.java index d5dc074d71d..84718294a66 100644 --- a/core/src/main/java/org/jruby/RubyKernel.java +++ b/core/src/main/java/org/jruby/RubyKernel.java @@ -63,6 +63,7 @@ import org.jruby.exceptions.CatchThrow; import org.jruby.exceptions.MainExitException; import org.jruby.exceptions.RaiseException; +import org.jruby.internal.runtime.GlobalVariables; import org.jruby.internal.runtime.methods.DynamicMethod; import org.jruby.internal.runtime.methods.JavaMethod.JavaMethodNBlock; import org.jruby.ir.interpreter.Interpreter; @@ -948,13 +949,12 @@ private static void exit(Ruby runtime, IRubyObject[] args, boolean hard) { */ @JRubyMethod(name = "global_variables", module = true, visibility = PRIVATE) public static RubyArray global_variables(ThreadContext context, IRubyObject recv) { - var globalVariables = newArray(context); + GlobalVariables globals = context.runtime.getGlobalVariables(); + var globalVariables = newRawArray(context, globals.size()); - for (String globalVariableName : context.runtime.getGlobalVariables().getNames()) { - globalVariables.append(context, asSymbol(context, globalVariableName)); - } + globals.eachName(context, globalVariables, (c, g, s) -> g.append(c, asSymbol(c, s))); - return globalVariables; + return globalVariables.finishRawArray(context); } /** diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java index d2bab507b2b..a7b11406ac8 100644 --- a/core/src/main/java/org/jruby/RubyModule.java +++ b/core/src/main/java/org/jruby/RubyModule.java @@ -4567,11 +4567,11 @@ public final void setConstantVisibility(Ruby runtime, String name, boolean hidde @JRubyMethod(name = "refinements") public IRubyObject refinements(ThreadContext context) { - var refinementModules = newArray(context); + var refinementModules = newRawArray(context, refinements.size()); refinements.forEach((key, value) -> refinementModules.append(context, value)); - return refinementModules; + return refinementModules.finishRawArray(context); } @JRubyMethod(name = "target") diff --git a/core/src/main/java/org/jruby/ext/ffi/AbstractMemory.java b/core/src/main/java/org/jruby/ext/ffi/AbstractMemory.java index 60005e6ba5e..92b5642fde2 100644 --- a/core/src/main/java/org/jruby/ext/ffi/AbstractMemory.java +++ b/core/src/main/java/org/jruby/ext/ffi/AbstractMemory.java @@ -51,6 +51,7 @@ import static org.jruby.api.Convert.*; import static org.jruby.api.Create.newArray; import static org.jruby.api.Create.newEmptyString; +import static org.jruby.api.Create.newRawArray; import static org.jruby.api.Error.*; /** @@ -1861,7 +1862,7 @@ public IRubyObject get_string(ThreadContext context, IRubyObject offArg, IRubyOb @JRubyMethod(name = { "get_array_of_string" }) public IRubyObject get_array_of_string(ThreadContext context, IRubyObject rbOffset) { final int POINTER_SIZE = (Platform.getPlatform().addressSize() / 8); - final var arr = newArray(context); + final var arr = newRawArray(context, (size - POINTER_SIZE) / POINTER_SIZE); for (long off = getOffset(rbOffset); off <= size - POINTER_SIZE; off += POINTER_SIZE) { final MemoryIO mem = getMemoryIO().getMemoryIO(off); @@ -1870,7 +1871,7 @@ public IRubyObject get_array_of_string(ThreadContext context, IRubyObject rbOffs arr.add(MemoryUtil.getTaintedString(context.runtime, mem, 0)); } - return arr; + return arr.finishRawArray(context); } @JRubyMethod(name = { "get_array_of_string" }) diff --git a/core/src/main/java/org/jruby/ext/ffi/jffi/VariadicInvoker.java b/core/src/main/java/org/jruby/ext/ffi/jffi/VariadicInvoker.java index 07563bf135e..e0734661ff9 100644 --- a/core/src/main/java/org/jruby/ext/ffi/jffi/VariadicInvoker.java +++ b/core/src/main/java/org/jruby/ext/ffi/jffi/VariadicInvoker.java @@ -24,6 +24,7 @@ import static org.jruby.api.Create.newArray; import static org.jruby.api.Convert.asSymbol; +import static org.jruby.api.Create.newRawArray; import static org.jruby.api.Error.typeError; @JRubyClass(name = "FFI::VariadicInvoker", parent = "Object") @@ -114,15 +115,17 @@ public static VariadicInvoker newInstance(ThreadContext context, IRubyObject kla CallingConvention callConvention = "stdcall".equals(convention) ? CallingConvention.STDCALL : CallingConvention.DEFAULT; - var fixed = newArray(context); + int length = paramTypes.getLength(); + var fixed = newRawArray(context, length); int fixedParamCount = 0; - for (int i = 0; i < paramTypes.getLength(); ++i) { + for (int i = 0; i < length; ++i) { Type type = (Type)paramTypes.entry(i); if (type.getNativeType() != org.jruby.ext.ffi.NativeType.VARARGS) { fixed.append(context, type); fixedParamCount++; } } + fixed.finishRawArray(context); FunctionInvoker functionInvoker = DefaultMethodFactory.getFunctionInvoker(returnType); diff --git a/core/src/main/java/org/jruby/ext/socket/RubyTCPSocket.java b/core/src/main/java/org/jruby/ext/socket/RubyTCPSocket.java index 42662109cc8..a7209fb5d59 100644 --- a/core/src/main/java/org/jruby/ext/socket/RubyTCPSocket.java +++ b/core/src/main/java/org/jruby/ext/socket/RubyTCPSocket.java @@ -228,7 +228,7 @@ public static IRubyObject gethostbyname(ThreadContext context, IRubyObject recv, return RubyArray.newArray(context.runtime, newString(context, do_not_reverse_lookup(context, recv).isTrue() ? addr.getHostAddress() : addr.getCanonicalHostName()), - newArray(context), + newEmptyArray(context), asFixnum(context, addr instanceof Inet4Address ? AF_INET.longValue() : AF_INET6.longValue()), newString(context, addr.getHostAddress())); } diff --git a/core/src/main/java/org/jruby/ext/socket/SocketUtils.java b/core/src/main/java/org/jruby/ext/socket/SocketUtils.java index 16ebb1df4b7..6505d8ba84c 100644 --- a/core/src/main/java/org/jruby/ext/socket/SocketUtils.java +++ b/core/src/main/java/org/jruby/ext/socket/SocketUtils.java @@ -99,7 +99,7 @@ public static IRubyObject gethostname(ThreadContext context) { public static IRubyObject gethostbyaddr(ThreadContext context, IRubyObject[] args) { var ret0 = newString(context, Sockaddr.addressFromString(context.runtime, args[0].convertToString().toString()).getCanonicalHostName()); - var ret1 = newArray(context); + var ret1 = newEmptyArray(context); var ret2 = asFixnum(context, 2); // AF_INET var ret3 = args[0]; @@ -148,7 +148,7 @@ public static IRubyObject gethostbyname(ThreadContext context, IRubyObject hostn return RubyArray.newArray(context.runtime, newString(context, addr.getCanonicalHostName()), - newArray(context), + newEmptyArray(context), asFixnum(context, AF_INET.longValue()), newString(context, new ByteList(addr.getAddress()))); } catch(UnknownHostException e) { diff --git a/core/src/main/java/org/jruby/internal/runtime/GlobalVariables.java b/core/src/main/java/org/jruby/internal/runtime/GlobalVariables.java index b79af709810..cf0e892934c 100644 --- a/core/src/main/java/org/jruby/internal/runtime/GlobalVariables.java +++ b/core/src/main/java/org/jruby/internal/runtime/GlobalVariables.java @@ -41,7 +41,9 @@ import org.jruby.common.IRubyWarnings.ID; import org.jruby.exceptions.RaiseException; import org.jruby.runtime.IAccessor; +import org.jruby.runtime.ThreadContext; import org.jruby.runtime.builtin.IRubyObject; +import org.jruby.util.func.TriConsumer; /** * @@ -163,6 +165,10 @@ public Set getNames() { return globalVariables.keySet(); } + public void eachName(ThreadContext context, State state, TriConsumer consumer) { + globalVariables.forEach((s, g) -> consumer.accept(context, state, s)); + } + private GlobalVariable createIfNotDefined(String name) { return globalVariables.computeIfAbsent(name, (n) -> GlobalVariable.newUndefined(runtime, n)); } @@ -176,4 +182,8 @@ public IRubyObject getDefaultSeparator() { public void setDefaultSeparator(IRubyObject defaultSeparator) { this.defaultSeparator = defaultSeparator; } + + public int size() { + return globalVariables.size(); + } } diff --git a/core/src/main/java/org/jruby/ir/interpreter/ExitableInterpreterEngine.java b/core/src/main/java/org/jruby/ir/interpreter/ExitableInterpreterEngine.java index 17db432cba8..cae966f7b6f 100644 --- a/core/src/main/java/org/jruby/ir/interpreter/ExitableInterpreterEngine.java +++ b/core/src/main/java/org/jruby/ir/interpreter/ExitableInterpreterEngine.java @@ -39,6 +39,7 @@ import org.jruby.runtime.builtin.IRubyObject; import static org.jruby.api.Create.newArray; +import static org.jruby.api.Create.newEmptyArray; import static org.jruby.api.Error.runtimeError; import static org.jruby.util.RubyStringBuilder.ids; import static org.jruby.util.RubyStringBuilder.str; @@ -113,7 +114,7 @@ public ExitableReturn interpret(ThreadContext context, Block block, IRubyObject break; case RET_OP: processReturnOp(context, block, instr, operation, currDynScope, temp, self, currScope); - return new ExitableReturn(newArray(context), Block.NULL_BLOCK); + return new ExitableReturn(newEmptyArray(context), Block.NULL_BLOCK); case BRANCH_OP: switch (operation) { case JUMP: diff --git a/core/src/main/java/org/jruby/javasupport/ext/JavaLang.java b/core/src/main/java/org/jruby/javasupport/ext/JavaLang.java index 87ac51f36ce..e400f796b93 100644 --- a/core/src/main/java/org/jruby/javasupport/ext/JavaLang.java +++ b/core/src/main/java/org/jruby/javasupport/ext/JavaLang.java @@ -548,41 +548,65 @@ public static IRubyObject declared_annotations_p(final ThreadContext context, fi @JRubyMethod public static IRubyObject java_instance_methods(final ThreadContext context, final IRubyObject self) { final java.lang.Class klass = unwrapJavaObject(self); - final var methods = newArray(context); - for (java.lang.reflect.Method method : klass.getMethods()) { - if (!Modifier.isStatic(method.getModifiers())) methods.add(method); + Method[] publicMethods = klass.getMethods(); + + // quick count for accurate size + int size = 0; + for (java.lang.reflect.Method method : publicMethods) if (!Modifier.isStatic(method.getModifiers())) size++; + final var methods = newRawArray(context, size); + + for (java.lang.reflect.Method method : publicMethods) { + if (!Modifier.isStatic(method.getModifiers())) methods.add(context, method); } - return methods; + return methods.finishRawArray(context); } @JRubyMethod public static IRubyObject declared_instance_methods(final ThreadContext context, final IRubyObject self) { final java.lang.Class klass = unwrapJavaObject(self); - final var methods = newArray(context); - for (java.lang.reflect.Method method : klass.getDeclaredMethods()) { - if (!Modifier.isStatic(method.getModifiers())) methods.add(method); + Method[] declaredMethods = klass.getDeclaredMethods(); + + // quick count for accurate size + int size = 0; + for (java.lang.reflect.Method method : declaredMethods) if (!Modifier.isStatic(method.getModifiers())) size++; + final var methods = newRawArray(context, size); + + for (java.lang.reflect.Method method : declaredMethods) { + if (!Modifier.isStatic(method.getModifiers())) methods.add(context, method); } - return methods; + return methods.finishRawArray(context); } @JRubyMethod public static IRubyObject java_class_methods(final ThreadContext context, final IRubyObject self) { final java.lang.Class klass = unwrapJavaObject(self); - final var methods = newArray(context); - for ( java.lang.reflect.Method method : klass.getMethods() ) { + Method[] publicMethods = klass.getMethods(); + + // quick count for accurate size + int size = 0; + for (java.lang.reflect.Method method : publicMethods) if (Modifier.isStatic(method.getModifiers())) size++; + final var methods = newRawArray(context, size); + + for ( java.lang.reflect.Method method : publicMethods) { if (Modifier.isStatic(method.getModifiers())) methods.add(method); } - return methods; + return methods.finishRawArray(context); } @JRubyMethod public static IRubyObject declared_class_methods(final ThreadContext context, final IRubyObject self) { final java.lang.Class klass = unwrapJavaObject(self); - final var methods = newArray(context); - for (java.lang.reflect.Method method : klass.getDeclaredMethods()) { + Method[] declaredMethods = klass.getDeclaredMethods(); + + // quick count for accurate size + int size = 0; + for (java.lang.reflect.Method method : declaredMethods) if (Modifier.isStatic(method.getModifiers())) size++; + final var methods = newRawArray(context, size); + + for (java.lang.reflect.Method method : declaredMethods) { if (Modifier.isStatic(method.getModifiers())) methods.add(method); } - return methods; + return methods.finishRawArray(context); } @JRubyMethod(name = "<=>") // Ruby Comparable diff --git a/core/src/main/java/org/jruby/parser/Parser.java b/core/src/main/java/org/jruby/parser/Parser.java index c4c21c3f8a1..fc9b571b6a7 100644 --- a/core/src/main/java/org/jruby/parser/Parser.java +++ b/core/src/main/java/org/jruby/parser/Parser.java @@ -69,6 +69,7 @@ import org.jruby.util.CommonByteLists; import static org.jruby.api.Create.newArray; +import static org.jruby.api.Create.newEmptyArray; import static org.jruby.api.Create.newString; import static org.jruby.parser.ParserType.*; @@ -225,7 +226,7 @@ protected RubyArray getLines(boolean isEvalParse, String file, int length) { if (!(scriptLines instanceof RubyHash)) return null; var context = runtime.getCurrentContext(); - var list = length == -1 ? newArray(context) : newArray(context, length); + var list = length == -1 ? newEmptyArray(context) : newArray(context, length); ((RubyHash) scriptLines).op_aset(context, newString(context, file), list); return list; } diff --git a/core/src/main/java/org/jruby/util/io/PopenExecutor.java b/core/src/main/java/org/jruby/util/io/PopenExecutor.java index a69a148e48c..565b92b0b09 100644 --- a/core/src/main/java/org/jruby/util/io/PopenExecutor.java +++ b/core/src/main/java/org/jruby/util/io/PopenExecutor.java @@ -372,21 +372,21 @@ static void execargSetenv(ThreadContext context, ExecArg eargp, IRubyObject env) public static RubyArray checkExecEnv(ThreadContext context, RubyHash hash, ExecArg pathArg) { var env = newArray(context); - for (Map.Entry entry : (Set>)hash.directEntrySet()) { - IRubyObject key = entry.getKey(); - IRubyObject val = entry.getValue(); - RubyString keyString = checkEmbeddedNulls(context, key).export(context); - String k = keyString.toString(); + hash.visitAll(context, new RubyHash.VisitorWithState() { + @Override + public void visit(ThreadContext context, RubyHash self, IRubyObject key, IRubyObject value, int index, RubyArray state) { + RubyString keyString = checkEmbeddedNulls(context, key).export(context); + String k = keyString.toString(); - if (k.indexOf('=') != -1) throw argumentError(context, "environment name contains a equal : " + k); + if (k.indexOf('=') != -1) throw argumentError(context, "environment name contains a equal : " + k); - if (!val.isNil()) val = checkEmbeddedNulls(context, val); - if (!val.isNil()) val = ((RubyString) val).export(context); + if (!value.isNil()) value = checkEmbeddedNulls(context, value); + if (!value.isNil()) value = ((RubyString) value).export(context); - if (k.equalsIgnoreCase("PATH")) pathArg.path_env = val; + if (k.equalsIgnoreCase("PATH")) pathArg.path_env = value; - env.push(context, newArray(context, keyString, val)); - } + state.push(context, newArray(context, keyString, value)); + }}, env); return env; }