RFR: 8232864: Classes generated by GenerateJLIClassesPlugin.java are not reproducable
Jie Fu
fujie at loongson.cn
Sat Oct 26 02:34:37 UTC 2019
Hi Mandy,
Thanks for your review and help.
I think your refactoring is a nice improvement.
Updated: http://cr.openjdk.java.net/~jiefu/8232864/webrev.03/
It seems much better now.
Testing:
- make test TEST="tier1 tier2 tier3" CONF=release
Thanks a lot.
Best regards,
Jie
On 2019/10/26 上午3:29, Mandy Chung wrote:
>
>
> On 10/23/19 11:21 PM, Jie Fu wrote:
>> Hi Mandy,
>>
>> Updated: http://cr.openjdk.java.net/~jiefu/8232864/webrev.02/
>> It seems better now.
>>
> It's better while I think this can be further simplified.
>
> line 101 to 217 involves iterating on the default sets/map and then
> create a new TreeSet and TreeMap and then process the config file.
>
> Currently addDMHMethodType validates the methodType and then adds
> to dmhMethods which ensures that the map entry value is a sorted set.
> So we can add one new method to add an entry to speciesTypes,
> invokeTypes and callSiteTypes which performs the validation and then
> add to the corresponding collection.
>
> I think that way the code will be cleaner. Below is something for
> your reference.
>
> Mandy
>
> diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/GenerateJLIClassesPlugin.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/GenerateJLIClassesPlugin.java
> --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/GenerateJLIClassesPlugin.java
> +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/GenerateJLIClassesPlugin.java
> @@ -75,13 +75,13 @@
> private static final JavaLangInvokeAccess JLIA
> = SharedSecrets.getJavaLangInvokeAccess();
>
> - Set<String> speciesTypes = Set.of();
> + private final TreeSet<String> speciesTypes = new TreeSet<>();
>
> - Set<String> invokerTypes = Set.of();
> + private final TreeSet<String> invokerTypes = new TreeSet<>();
>
> - Set<String> callSiteTypes = Set.of();
> + private final TreeSet<String> callSiteTypes = new TreeSet<>();
>
> - Map<String, Set<String>> dmhMethods = Map.of();
> + private final Map<String, Set<String>> dmhMethods = new TreeMap<>();
>
> String mainArgument;
>
> @@ -187,21 +187,31 @@
> mainArgument = config.get(NAME);
> }
>
> + private void addSpeciesType(String type) {
> + speciesTypes.add(expandSignature(type));
> + }
> +
> + private void addInvokerType(String methodType) {
> + validateMethodType(methodType);
> + invokerTypes.add(methodType);
> + }
> +
> + private void addCallSisterType(String csType) {
> + validateMethodType(csType);
> + callSiteTypes.add(csType);
> + }
> +
> public void initialize(ResourcePool in) {
> // Start with the default configuration
> - speciesTypes = defaultSpecies().stream()
> - .map(type -> expandSignature(type))
> - .collect(Collectors.toSet());
> + defaultSpecies().stream().forEach(this::addSpeciesType);
>
> - invokerTypes = defaultInvokers();
> - validateMethodTypes(invokerTypes);
> + defaultInvokers().stream().forEach(this::validateMethodType);
>
> - callSiteTypes = defaultCallSiteTypes();
> + defaultCallSiteTypes().stream().forEach(this::addCallSisterType);
>
> - dmhMethods = defaultDMHMethods();
> - for (Set<String> dmhMethodTypes : dmhMethods.values()) {
> - validateMethodTypes(dmhMethodTypes);
> - }
> + defaultDMHMethods().entrySet().stream().forEach(e -> {
> + e.getValue().stream().forEach(type -> addDMHMethodType(e.getKey(), type))
> + });
>
> // Extend the default configuration with the contents in the supplied
> // input file - if none was supplied we look for the default file
> @@ -225,18 +235,6 @@
> }
>
>
> private void readTraceConfig(Stream<String> lines) {
> - // Use TreeSet/TreeMap to keep things sorted in a deterministic
> - // order to avoid scrambling the layout on small changes and to
> - // ease finding methods in the generated code
> - speciesTypes = new TreeSet<>(speciesTypes);
> - invokerTypes = new TreeSet<>(invokerTypes);
> - callSiteTypes = new TreeSet<>(callSiteTypes);
> -
> - TreeMap<String, Set<String>> newDMHMethods = new TreeMap<>();
> - for (Map.Entry<String, Set<String>> entry : dmhMethods.entrySet()) {
> - newDMHMethods.put(entry.getKey(), new TreeSet<>(entry.getValue()));
> - }
> - dmhMethods = newDMHMethods;
> lines.map(line -> line.split(" "))
> .forEach(parts -> {
> switch (parts[0]) {
> @@ -245,19 +243,18 @@
> if (parts.length == 3 && parts[1].startsWith("java.lang.invoke.BoundMethodHandle$Species_")) {
> String species = parts[1].substring("java.lang.invoke.BoundMethodHandle$Species_".length());
> if (!"L".equals(species)) {
> - speciesTypes.add(expandSignature(species));
> + addSpeciesType(species);
> }
> }
> break;
> case "[LF_RESOLVE]":
> String methodType = parts[3];
> - validateMethodType(methodType);
> if (parts[1].equals(INVOKERS_HOLDER_NAME)) {
> if ("linkToTargetMethod".equals(parts[2]) ||
> "linkToCallSite".equals(parts[2])) {
> - callSiteTypes.add(methodType);
> + addCallSisterType(methodType);
> } else {
> - invokerTypes.add(methodType);
> + addInvokerType(methodType);
> }
> } else if (parts[1].contains("DirectMethodHandle")) {
> String dmh = parts[2];
> @@ -291,12 +288,6 @@
> }
> }
>
> - private void validateMethodTypes(Set<String> dmhMethodTypes) {
> - for (String type : dmhMethodTypes) {
> - validateMethodType(type);
> - }
> - }
>
> private void validateMethodType(String type) {
> String[] typeParts = type.split("_");
> // check return type (second part)
> @@ -338,12 +329,6 @@
>
> // Generate LambdaForm Holder classes
> generateHolderClasses(out);
> -
> - // Let it go
> - speciesTypes = null;
> - invokerTypes = null;
> - dmhMethods = null;
> -
> return out.build();
> }
>
More information about the core-libs-dev
mailing list