On Wed, 16 Sep 2020 19:01:18 GMT, Ioi Lam <ik...@openjdk.org> wrote: >> This patch is reorganized after 8252725, which is separated from this patch >> to refactor jlink glugin code. The previous >> webrev with hg can be found at: >> http://cr.openjdk.java.net/~minqi/2020/8247536/webrev-05. With 8252725 >> integrated, the >> regeneration of holder classes is simply to call the new added >> GenerateJLIClassesHelper.cdsGenerateHolderClasses >> function. Tests: tier1-4 > > src/java.base/share/classes/java/lang/invoke/GenerateJLIClassesHelper.java > line 67: > >> 65: if (VM.isDumpLoadedClassListSetAndOpen) { >> 66: VM.cdsTraceResolve(traceLF); >> 67: } > > GenerateJLIClassesHelper shouldn't need to know why the trace is needed. > Also, "cdsTraceResolve" is too generic. > > I think it's better to have > if (TRACE_RESOLVE || VM.CDS_TRACE_JLINV_RESOLVE) { > ... > VM.cdsTraceJLINVResolve(traceLF); > > The acronym JLINV is used in > [methodHandles.cpp](https://github.com/openjdk/jdk/blob/ce93cbce77e1f4baa52676826c8ae27d474360b6/src/hotspot/share/prims/methodHandles.cpp#L1524)
With CDS related code moved to CDS.java, I think we should keep TRACE_RESOLVE here. A new name like suggested by Mandy, logTraceResolve in CDS.java > src/java.base/share/classes/jdk/internal/misc/VM.java line 490: > >> 488: */ >> 489: public static boolean isDumpLoadedClassListSetAndOpen; >> 490: private static native boolean isDumpLoadedClassListSetAndOpen0(); > > I would suggest to rename to `isDumpingLoadedClassList` Will change. ------------- PR: https://git.openjdk.java.net/jdk/pull/193
