RFR: 8247666: Support Lambda proxy classes in static CDS archive [v5]

[Calvin Cheung]

On Tue, 13 Oct 2020 23:59:38 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> 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)
>
> 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?

Those new includes are not needed. I've removed them.

> 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();
> }

I've added the ClassListWriter.hpp and changed other classes which operate on the classlist.

> 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.`

Fixed.

> 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.

I've changed the API's in CDS.java to be consistent with CDS.isDumpingClasslist.

-------------

PR: https://git.openjdk.java.net/jdk/pull/364