Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8264288: Performance issue with MethodHandle.asCollector #3306

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -870,12 +870,10 @@ void addMethod() {
i += 2; // jump to the end of the LOOP idiom
continue;
case NEW_ARRAY:
Class<?> rtype = name.function.methodType().returnType();
if (isStaticallyNameable(rtype)) {
JornVernee marked this conversation as resolved.
Show resolved Hide resolved
emitNewArray(name);
continue;
}
break;
// component type argument needs to be constant
if (!(name.arguments[0] instanceof Class<?>)) break;
emitNewArray(name);
continue;
case ARRAY_LOAD:
emitArrayLoad(name);
continue;
Expand Down Expand Up @@ -936,6 +934,22 @@ static final class BytecodeGenerationException extends RuntimeException {
}
}

void emitNewArray(Name name) {
// call to Array.newInstance. Get the component type, which should be constant for this intrinsic
Class<?> rtype = name.function.methodType().returnType();
Class<?> arrayElementType = (Class<?>) name.arguments[0];
// push array length
emitPushArgument(name, 1);
if (!arrayElementType.isPrimitive()) {
mv.visitTypeInsn(Opcodes.ANEWARRAY, getInternalName(arrayElementType));
} else {
byte tc = arrayTypeCode(Wrapper.forPrimitiveType(arrayElementType));
mv.visitIntInsn(Opcodes.NEWARRAY, tc);
}
// the array is left on the stack
assertStaticType(rtype, name);
}

void emitArrayLoad(Name name) { emitArrayOp(name, Opcodes.AALOAD); }
void emitArrayStore(Name name) { emitArrayOp(name, Opcodes.AASTORE); }
void emitArrayLength(Name name) { emitArrayOp(name, Opcodes.ARRAYLENGTH); }
Expand Down Expand Up @@ -1112,43 +1126,6 @@ void emitStaticInvoke(MemberName member, Name name) {
}
}

void emitNewArray(Name name) throws InternalError {
Class<?> rtype = name.function.methodType().returnType();
if (name.arguments.length == 0) {
// The array will be a constant.
Object emptyArray;
try {
emptyArray = name.function.resolvedHandle().invoke();
} catch (Throwable ex) {
throw uncaughtException(ex);
}
assert(java.lang.reflect.Array.getLength(emptyArray) == 0);
assert(emptyArray.getClass() == rtype); // exact typing
mv.visitFieldInsn(Opcodes.GETSTATIC, className, classData(emptyArray), "Ljava/lang/Object;");
emitReferenceCast(rtype, emptyArray);
return;
}
Class<?> arrayElementType = rtype.getComponentType();
assert(arrayElementType != null);
emitIconstInsn(name.arguments.length);
int xas = Opcodes.AASTORE;
if (!arrayElementType.isPrimitive()) {
mv.visitTypeInsn(Opcodes.ANEWARRAY, getInternalName(arrayElementType));
} else {
byte tc = arrayTypeCode(Wrapper.forPrimitiveType(arrayElementType));
xas = arrayInsnOpcode(tc, xas);
mv.visitIntInsn(Opcodes.NEWARRAY, tc);
}
// store arguments
for (int i = 0; i < name.arguments.length; i++) {
mv.visitInsn(Opcodes.DUP);
emitIconstInsn(i);
emitPushArgument(name, i);
mv.visitInsn(xas);
}
// the array is left on the stack
assertStaticType(rtype, name);
}
int refKindOpcode(byte refKind) {
switch (refKind) {
case REF_invokeVirtual: return Opcodes.INVOKEVIRTUAL;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,7 @@ enum Kind {
PUT_DOUBLE_VOLATILE("putDoubleVolatile"),
TRY_FINALLY("tryFinally"),
COLLECT("collect"),
COLLECTOR("collector"),
CONVERT("convert"),
SPREAD("spread"),
LOOP("loop"),
Expand Down
51 changes: 0 additions & 51 deletions src/java.base/share/classes/java/lang/invoke/LambdaFormEditor.java
Original file line number Diff line number Diff line change
Expand Up @@ -648,57 +648,6 @@ LambdaForm collectArgumentsForm(int pos, MethodType collectorType) {
return putInCache(key, form);
}

LambdaForm collectArgumentArrayForm(int pos, MethodHandle arrayCollector) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's counter-intuitive that removing a LFE tactic would be harmless.
Each LFE tactic is a point where LFs can be shared, reducing footprint.
But in this case collectArgumentArrayForm is always paired with
collectArgumentsForm, so the latter takes care of sharing.
The actual code which makes the arrays is also shared, via
Makers.TYPED_COLLECTORS (unchanged).

MethodType collectorType = arrayCollector.type();
int collectorArity = collectorType.parameterCount();
assert(arrayCollector.intrinsicName() == Intrinsic.NEW_ARRAY);
Class<?> arrayType = collectorType.returnType();
Class<?> elementType = arrayType.getComponentType();
BasicType argType = basicType(elementType);
int argTypeKey = argType.ordinal();
if (argType.basicTypeClass() != elementType) {
// return null if it requires more metadata (like String[].class)
if (!elementType.isPrimitive())
return null;
argTypeKey = TYPE_LIMIT + Wrapper.forPrimitiveType(elementType).ordinal();
}
assert(collectorType.parameterList().equals(Collections.nCopies(collectorArity, elementType)));
byte kind = COLLECT_ARGS_TO_ARRAY;
TransformKey key = TransformKey.of(kind, pos, collectorArity, argTypeKey);
LambdaForm form = getInCache(key);
if (form != null) {
assert(form.arity == lambdaForm.arity - 1 + collectorArity);
return form;
}
LambdaFormBuffer buf = buffer();
buf.startEdit();

assert(pos + 1 <= lambdaForm.arity);
assert(pos > 0); // cannot filter the MH arg itself

Name[] newParams = new Name[collectorArity];
for (int i = 0; i < collectorArity; i++) {
newParams[i] = new Name(pos + i, argType);
}
Name callCombiner = new Name(new NamedFunction(arrayCollector, Intrinsic.NEW_ARRAY),
(Object[]) /*...*/ newParams);

// insert the new expression
int exprPos = lambdaForm.arity();
buf.insertExpression(exprPos, callCombiner);

// insert new arguments
int argPos = pos + 1; // skip result parameter
for (Name newParam : newParams) {
buf.insertParameter(argPos++, newParam);
}
assert(buf.lastIndexOf(callCombiner) == exprPos+newParams.length);
buf.replaceParameterByCopy(pos, exprPos+newParams.length);

form = buf.endEdit();
return putInCache(key, form);
}

LambdaForm filterArgumentForm(int pos, BasicType newType) {
TransformKey key = TransformKey.of(FILTER_ARG, pos, newType.ordinal());
LambdaForm form = getInCache(key);
Expand Down
10 changes: 3 additions & 7 deletions src/java.base/share/classes/java/lang/invoke/MethodHandle.java
Original file line number Diff line number Diff line change
Expand Up @@ -1244,13 +1244,9 @@ public MethodHandle asCollector(int collectArgPos, Class<?> arrayType, int array
asCollectorChecks(arrayType, collectArgPos, arrayLength);
BoundMethodHandle mh = rebind();
MethodType resultType = type().asCollectorType(arrayType, collectArgPos, arrayLength);
MethodHandle newArray = MethodHandleImpl.varargsArray(arrayType, arrayLength);
LambdaForm lform = mh.editor().collectArgumentArrayForm(1 + collectArgPos, newArray);
if (lform != null) {
return mh.copyWith(resultType, lform);
}
lform = mh.editor().collectArgumentsForm(1 + collectArgPos, newArray.type().basicType());
return mh.copyWithExtendL(resultType, lform, newArray);
MethodHandle collector = MethodHandleImpl.varargsArray(arrayType, arrayLength);
LambdaForm lform = mh.editor().collectArgumentsForm(1 + collectArgPos, collector.type().basicType());
return mh.copyWithExtendL(resultType, lform, collector);
}

/**
Expand Down
Loading