On Thu, 29 Jun 2023 20:58:53 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>> Oliver Kopp has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix threshold > > 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) ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/14408#discussion_r1248764698
