RFR: 8247666: Support Lambda proxy classes in static CDS archive [v5]
Ioi Lam
iklam at openjdk.java.net
Wed Oct 14 00:43:12 UTC 2020
On Tue, 13 Oct 2020 23:08:22 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:
>> Following up on archiving lambda proxy classes in dynamic CDS archive
>> ([JDK-8198698](https://bugs.openjdk.java.net/browse/JDK-8198698)), this RFE adds the functionality of archiving of
>> lambda proxy classes in static CDS archive.
>> When the -XX:DumpLoadedClassList is enabled, the constant pool index related to LambdaMetafactory that are resolved
>> during application execution will be included in the classlist. The entry for a lambda proxy class in a class list will
>> be of the following format:
>> `@lambda-proxy: <classname> <cp index>`
>>
>> e.g.
>> `@lambda-proxy: test/java/lang/invoke/MethodHandlesGeneralTest 233`
>> `@lambda-proxy: test/java/lang/invoke/MethodHandlesGeneralTest 355`
>>
>> When dumping a CDS archive using the -Xshare:dump and -XX:ExtraSharedClassListFile options, when the above
>> `@lambda-proxy` entry is encountered while parsing the classlist, we will resolve the corresponding constant pool
>> indices (233 and 355 in the above example). As a result, lambda proxy classes will be generated for the constant pool
>> entries, and will be cached using a similar mechanism to JDK-8198698. During dumping, there is check on the cp index
>> and on the created BootstrapInfo using the cp index. VM will exit with an error message if the check has failed.
>> During runtime when looking up a lambda proxy class, the lookup will be perform on the static CDS archive and if not
>> found, then lookup from the dynamic archive if one is specified. (Only name change (IsDynamicDumpingEnabled ->
>> IsCDSDumpingEnabled) is involved in the core-libs code.)
>> Testing: tiers 1,2,3,4.
>>
>> Performance results (javac on HelloWorld on linux-x64):
>> Results of " perf stat -r 40 bin/javac -J-Xshare:on -J-XX:SharedArchiveFile=javac2.jsa Bench_HelloWorld.java "
>> 1: 2228016795 2067752708 (-160264087) ----- 377.760 349.110 (-28.650) -----
>> 2: 2223051476 2063016483 (-160034993) ----- 374.580 350.620 (-23.960) ----
>> 3: 2225908334 2067673847 (-158234487) ----- 375.220 350.990 (-24.230) ----
>> 4: 2225835999 2064596883 (-161239116) ----- 374.670 349.840 (-24.830) ----
>> 5: 2226005510 2061694332 (-164311178) ----- 373.512 351.120 (-22.392) ----
>> 6: 2225574949 2062657482 (-162917467) ----- 374.710 348.380 (-26.330) -----
>> 7: 2224702424 2064634122 (-160068302) ----- 373.670 349.510 (-24.160) ----
>> 8: 2226662277 2066301134 (-160361143) ----- 375.350 349.790 (-25.560) ----
>> 9: 2226761470 2063162795 (-163598675) ----- 374.260 351.290 (-22.970) ----
>> 10: 2230149089 2066203307 (-163945782) ----- 374.760 350.620 (-24.140) ----
>> ============================================================
>> 2226266109 2064768307 (-161497801) ----- 374.848 350.126 (-24.722) ----
>> instr delta = -161497801 -7.2542%
>> time delta = -24.722 ms -6.5951%
>
> Calvin Cheung has updated the pull request with a new target base due to a merge or a rebase. The pull request now
> contains ten commits:
> - fix minimal vm build issues
> - Merge branch 'master' into 8247666
> - Merge branch 'master' into 8247666
> - 1. Symbolic encoding of lambda proxy resolution. An example of @lambda-proxy in the classlist:
> @lambda-proxy: LambHello run ()Ljava/lang/Runnable; ()V 6 LambHello lambdabash ()V ()V
> 2. Removed BadCPIndex.java; added LambdaProxyClassList.java test.
> 3. Mandy's comments.
> - Merge branch 'master' into 8247666
> - exclude archived lambda proxy classes in the classlist
> - updated systemDictionaryShared.[c|h]pp based on suggestions from Ioi
> - fix extraneous whitespace
> - 8247666 (initial commit)
Changes requested by iklam (Reviewer).
src/hotspot/share/interpreter/linkResolver.cpp line 34:
> 32: #include "classfile/symbolTable.hpp"
> 33: #include "classfile/systemDictionary.hpp"
> 34: #include "classfile/systemDictionaryShared.hpp"
Are all the new includes necessary?
src/hotspot/share/memory/archiveUtils.cpp line 321:
> 319: }
> 320: }
> 321: }
I think if two threads try call ArchiveUtils::log_to_classlist at the same time, the output may be interleaved.
classlist_file is a fileStream, which uses fwrite to write to the file. In theory, if you write the whole line at once,
the output should be thread safe (at least on POSIX and Windows). But in your case, you would need to first get the
whole line into a buffer, and then write it out at once.
I think it would be safer, and more convenient, to use a lock to ensure thready safety. Maybe we can convert all uses
of classlist_file->print() to something like
class ClassListWriter {
static fileStream* _classlist_file;
MutexLocker locker;
public:
outputStream* stream() {
return _classlist_file;
}
static bool is_enabled() {
return _classlist_file != NULL && _classlist_file->is_open();
}
ClassListWriter() : locker(Thread::current(), ClassListFile_lock) {}
static void init() {
// classlist_file init code from ostrea.cpp
}
};
// WAS if (DumpLoadedClassList != NULL && classlist_file->is_open()) {
if (ClassListWriter::is_enabled()) {
ClassListWriter w;
w->stream()->print("aaaa");
w->stream()->print("bbbb");
w->stream()->cr();
}
src/hotspot/share/oops/instanceKlass.cpp line 4212:
> 4210: assert(stream == NULL, "shared class with stream");
> 4211: if (is_hidden()) {
> 4212: // Not including archived lambda proxy class in the classlist.
I think it's clearer to say `// Don't include archived lambda proxy class in the classlist.`
src/java.base/share/classes/jdk/internal/misc/CDS.java line 78:
> 76: * Check if CDS dumping is enabled via the DynamicDumpSharedSpaces or the DumpSharedSpaces flag.
> 77: */
> 78: public static native boolean isDumpingEnabled();
I think it will be more consistent if we use the same pattern as `CDS::isDumpingClassList()`
private static final boolean isDumpingArchive;
static {
isDumpingClassList = isDumpingArchive0();
}
/**
* Is the VM writing to a (static or dynamic) CDS archive.
*/
public static boolean isDumpingArchive() {
return isDumpingArchive;
}
Then in LambdaProxyClassArchive.java, there's no need to keep a separate variable of dumpArchive. You can simply call
CDS.isDumpingArchive(). The JIT compiler will automatically inline the call so it will be just as fast.
LambdaProxyClassArchive::sharingEnabled should also be rewritten to use this pattern.
-------------
PR: https://git.openjdk.java.net/jdk/pull/364
More information about the core-libs-dev
mailing list