On 2016-08-08 14:28, Aleksey Shipilev wrote:
On 08/08/2016 02:46 PM, Claes Redestad wrote:
Hi,
please review this change to add the ability to generate
DirectMethodHandles to the --generate-jli-classes jlink plugin.
The implementation generates all the specified DMHs as methods into a
single class, java.lang.invoke.DirectMethodHandle$DMH. At runtime when a
DMH's LF is set up, we speculatively resolve the member from this class
instead of generating and loading the bytecode as a distinct anonymous
class. This avoids loading a potentially large number of anonymous
classes at runtime, and also enables other startup optimizations such as
allowing CDS to see and dump this class to the shared archive.
webrev: http://cr.openjdk.java.net/~redestad/8163369/webrev.01/
DirectMethodHandle:
* DMH placeholder class might better be called
DirectMethodHandle.Holder, or something else that clearly spells out the
intent?
Sure.
InvokerBytecodeGenerator:
* Both generateNamedFunctionInvokerImpl() and
generateLambdaFormInterpreterEntryPointBytes() have methodEpilog() call,
but no methodPrologue()? classFilePrologue() used to do the prologue.
Nice catch, there was even 2 failed tests I had missed due to this.
GenerateJLIClassesPlugin:
* Do we actually need this block in this form?
// DirectMethodHandles
// Enable by default
boolean dmhEnabled = true;
if (mainArgument != null) {
Set<String> args = Arrays.stream(mainArgument.split(","))
.collect(Collectors.toSet());
if (!args.contains(DMH_PARAM)) {
dmhEnabled = false;
}
}
seems equivalent to a simpler:
boolean dmhEnabled = true;
if (mainArgument != null) {
List<String> args = Arrays.asList(mainArgument.split(","));
if (!args.contains(DMH_PARAM)) {
dmhEnabled = false;
}
}
You can even split the mainArgument once and reuse throughout configure()?
Fixed.
* This block does not check the first argument as the comment suggests,
only checks the second group:
String[] typeParts = type.split("_");
if (typeParts.length != 2 || typeParts[1].length() != 1
|| "LJIFDV".indexOf(typeParts[1].charAt(0)) == -1) {
throw new PluginException(
"Method type signature must be of form [LJIFD]*_[LJIFDV]");
}
I'm checking the return type here (the char after _ in "LL_L") and
then checking the first part inside expandSignature. Clarified the
intent with comments.
* primitiveType('Z') would throw a misleading "Not a primitive: Z"
Yes, added special casing for sub-int primitives (B, S, Z, C)
to indicate that 'I' should be used instead.
* This can iterate over values only:
for (Map.Entry<String, List<String>> entry : dmhMethods.entrySet()) {
count += entry.getValue().size();
}
Sure.
* I don't know the module visibility rules between jlink and java.base,
but can we reference DirectMethodHandle.DMH class as literal from the
plugin?
No, while java.lang.invoke is exported, DirectMethodHandle is
package-private
which prohibits static visibility from the plugin.
* requireBasicType(last) is loop invariant here:
for (int j = 1; j < count; j++) {
requireBasicType(last);
sb.append(last);
}
Sure.
http://cr.openjdk.java.net/~redestad/8163369/webrev.02/
/Claes
Thanks,
-Aleksey
