On Thu, 19 Nov 2020 19:42:51 GMT, Yumin Qi <[email protected]> wrote:
>> Before this change, the setting of the
>> `jdk.internal.lambda.disableEagerInitialization` property was not captured
>> during dumping of lambda proxy classes. There's a workaround in
>> `LambdaProxyClassArchive.find`, it won't call `findFromArchive` if the above
>> property is set.
>>
>> This change adds handling of the
>> `jdk.internal.lambda.disableEagerInitialization` property, specifically:
>>
>> - remove the above workaround;
>>
>> - capture the setting of the property in the archive header during CDS dump
>> time;
>>
>> - during runtime when finding an archived lambda proxy class, the setting of
>> the property will be compared with the stored value in the archive header.
>> If the values don't match, the archived lambda proxy class won't be used.
>>
>> Tests:
>>
>> - [x] ran all cds tests locally on linux-x64
>>
>> - [x] ran the `hotspot_appcds_dynamic` test group with
>> `-Dtest.dynamic.cds.archive=true` on linux-x64
>>
>> - [x] mach5 tiers 1,2,3 (in progress)
>
> src/hotspot/share/memory/metaspaceShared.cpp line 76:
>
>> 74: #endif
>> 75:
>> 76: #include <string.h>
>
> This include is strange, usually we do not include std head file in .cpp file.
The include is needed for strcasecmp and _stricmp.
If you do a grep like `find . -name "*.cpp" | xargs grep "#include <"`, you'll
find many .cpp files include std header files.
> src/hotspot/share/memory/metaspaceShared.cpp line 1821:
>
>> 1819: }
>> 1820:
>> 1821: void MetaspaceShared::set_disable_eager_init(const char* value) {
>
> strcasecmp is not platform dependent, why not use it for all? It does not
> need <string.h> included.
According to https://en.wikipedia.org/wiki/C_string_handling, strcasecmp is for
POSIX, BSD, stricmp is for Windows.
> test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/LambdaEagerInit.java
> line 35:
>
>> 33: * @library /test/lib /test/hotspot/jtreg/runtime/cds/appcds
>> 34: * @compile
>> ../../../../../../jdk/java/lang/invoke/lambda/LambdaEagerInitTest.java
>> 35: * @build sun.hotspot.WhiteBox
>
> Do we need WB here?
Our current design is that every test under the `dynamicArchive` dir extends
`DynamicArchiveTestBase` which depends on `sun.hotspot.WhiteBox`.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1301
