On Sat, 1 Jul 2023 10:38:05 GMT, Oliver Kopp <d...@openjdk.org> wrote:
>> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModulesPlugin.java >> line 691: >> >>> 689: } >>> 690: >>> 691: List<List<ModuleInfo>> splitModuleInfos = new >>> ArrayList<>(); >> >> Please add the comments to describe what this does. Pseudo code may also >> help. It'd be helpful to explain the reason why the Sub method stores and >> restores what local variables. >> >> I also wonder if you consider `moduleDescriptors` does this: >> >> ArrayList<Object> localVars = new ArrayList<>(); >> moduleDescriptorsSub0(moduleInfos, localVars); >> .... // add new local variables to localVars >> moduleDescriptorsSub1(moduleInfos, localVars); >> .... // add new local variables to localVars >> : >> : >> >> >> The same `localVars` array list is used and just add the new local variables >> defined from each batch (no need to create a new ArrayList and stores local >> variables every time) > > I assume the "old" comments were too detailed. I removed them at > [JabRef/jdk@`23bbc0c` > (#2)](https://github.com/JabRef/jdk/pull/2/commits/23bbc0ce0c8fd8a4cd689c0260c5fbcb91b20046) > to have the code reviewable. I can readd some of them if that helps. > > I also added a compressed description of the idea at [`edd85c9` > (#14408)](https://github.com/openjdk/jdk/pull/14408/commits/edd85c996125ce0a2950b0cfb95d1e387c35d5ff). > **Update** here, in the PR one simply needs to scroll up and sees the new > comment. Have you considered passing the same ArrayList for saving and restoring local variables? Currently each method creates one new array list and save the local variables from `firstVariableForDedup` to `nextLocalVar`, but the local variables from `firstVariableForDedup` are already added to the array list created by the previous method. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/14408#discussion_r1251088902
