RFR: 8232864: Classes generated by GenerateJLIClassesPlugin.java are not reproducable
Jie Fu
fujie at loongson.cn
Wed Oct 30 00:49:02 UTC 2019
Nice catch!
Thank you so much, Mandy.
On 2019/10/30 上午1:19, Mandy Chung wrote:
> Hi Jie,
>
> This looks good but I ran into test/jdk/tools/jlink/BasicTest failure
> with OOME in some system configuration. I made a minor fix to the
> cleanup of the configuration collections after the plugin executes.
>
> // Let it go
> - speciesTypes = null;
> - invokerTypes = null;
> - dmhMethods = null;
> + speciesTypes.clear();
> + invokerTypes.clear();
> + callSiteTypes.clear();
> + dmhMethods.clear();
>
> I have pushed the patch with the above change.
>
> Mandy
>
> On 10/25/19 7:34 PM, Jie Fu wrote:
>>
>> 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