RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete

Erik Joelsson erik.joelsson at oracle.com
Wed Apr 25 15:30:25 UTC 2018


Hello,

I would prefer if the .tmp file was not deleted. That will make it 
easier to debug problems in the future. We have recently had reason to 
look at the contents of the files here to figure out if the generator 
ran properly. If leaving it around, then perhaps call it .raw or 
something less temporary sounding than .tmp?

/Erik


On 2018-04-25 02:17, Magnus Ihse Bursie wrote:
>
>
> On 2018-04-25 08:17, David Holmes wrote:
>> cc'ing build-dev for the makefile change
>>
>> Hi Jiangli,
>>
>> On 25/04/2018 10:53 AM, Jiangli Zhou wrote:
>>> Please review the following changes that streamline the support for 
>>> application class data sharing by eliminating the requirement of 
>>> using -XX:+UseAppCDS to enable the feature. The support for 
>>> application class data sharing is now enabled transparently when 
>>> application classes or platform classes present in the classlist or 
>>> generated archive with -Xshare:dump/on/auto.
>>>
>>>     RFE: https://bugs.openjdk.java.net/browse/JDK-8193213
>>>     Bug: https://bugs.openjdk.java.net/browse/JDK-8182731
>>
>> These issues are not publicly visible, can you change them to be so?
>>
>>>     webrev: 
>>> http://cr.openjdk.java.net/~jiangli/8193213_8182731/webrev.00/
> Build changes look good.
>
> /Magnus
>>
>> Overall this seems fine. Thanks for explaining the logic changes 
>> below - much appreciated!
>>
>> One query: why was UseAppCDS not removed from the tests (or at least 
>> the tests you were modifying anyway) ? They will all need updating 
>> before 12 when the flag is expired.
>>
>> test/hotspot/jtreg/runtime/appcds/SharedArchiveFile.java
>>
>> Test 2 reference to UseAppCDS seems unnecessary now.
>> Test 3 is not needed as you're not using any diagnostic flag now.
>> Test 4 seems unnecessary now.
>>
>> test/hotspot/jtreg/runtime/appcds/TestCommon.java
>>
>> makeCommandLineForAppCDS should just be removed (yes that will cause 
>> some churn in the other tests) - but this can be a follow up test 
>> cleanup.
>>
>> test/hotspot/jtreg/runtime/appcds/UseAppCDS.java
>>
>> This test no longer makes much sense to me. The only tests should be 
>> that app classes get archived and used. It makes no sense to claim to 
>> be testing -XX:+UseAppCDS when that flag is obsolete. Likewise now 
>> that it is obsolete you don't need to test that -XX:-UseAppCDS has no 
>> affect.
>>
>> Thanks,
>> David
>>
>>
>>> Highlights of the changes:
>>> * UseAppCDS is obsolete in JDK 11
>>> * Remove the +UnlockCommercialFeatures requirement when using 
>>> application class data sharing in OracleJDK binary
>>> * SharedArchiveFile is a product flag for all configurations in JDK 11
>>> * DumpLoadedClassList produces a complete classlist including all 
>>> loaded library and application classes
>>>
>>> Thanks David Holmes for his guidance on CSR process.
>>>
>>> Following are the details for the VM and test changes:
>>> - Removed various ‘if (UseAppCDS)’ checks and UseAppCDS asserts.
>>>
>>> - Removed some of the CDS/AppCDS specifics from general class 
>>> loading code.
>>>
>>> - Removed scattered checks for non-empty directory. Dump time 
>>> non-empty directory check is done at one central place, 
>>> FileMapInfo::check_nonempty_dir_in_shared_path_table() after loading 
>>> all classes on the classlist. The behavior is backwards compatible:
>>>    - If no app/platform classes are loaded, dump time only reports 
>>> error if non-empty directory exists in -Xbootclasspath/a path
>>>    - If app/platform classes are loaded, dump time reports error for 
>>> any non-empty directory exists in -Xbootclasspath/a path, class 
>>> path, or module path
>>>
>>> - Removed unnecessary ‘if (!DumpSharedSpaces)’ from 
>>> SharedClassUtil::initialize(). The code path is only executed when 
>>> UseSharedSpaces is enabled.
>>>
>>> - Return immediately in 
>>> SystemDictionaryShared::find_or_load_shared_class() if the archive 
>>> header indicates there is no app or platform classes in the shared 
>>> archive. This reduces overhead in class loading if the shared 
>>> archive only contains system classes. It also provides backwards 
>>> compatibility in cases where shared application classes should be 
>>> disabled. For example, when the default system class loader is 
>>> replaced by user provided loader, archived application classes are 
>>> inactive (not loaded from archive) at runtime without affecting the 
>>> use of archived system classes.
>>>
>>> - Changed GenerateLinkOptData.gmk to filter out HelloClasslist from 
>>> the default classlist in JDK image, which is generated using 
>>> DumpLoadedClassList option. Thanks for David and Ioi's input on this.
>>>
>>> - Updated various cds/appcds tests to reflect the JVM changes. The 
>>> use of -XX:+UseAppCDS in tests remains unchanged.
>>>
>>>    - Removed -XX:+UnlockCommercialFeatures for -XX:+UseAppCDS in tests
>>>
>>>    - Removed -XX:+UnlockDiagnosticVMOptions for 
>>> -XX:SharedArchiveFile in tests
>>>
>>>    - Updated NonBootLoaderClasses.java: ensure app/platform classes 
>>> on classlist are archived without -XX:+UseAppCDS
>>>
>>>    - Updated DirClasspathTest.java: reflect the change for non-empty 
>>> directory handling
>>>
>>>    - Updated MismatchedUseAppCDS.java:  -XX:-UseAppCDS has no effect
>>>
>>> Tested with all cds and appcds tests locally with both fastdebug and 
>>> release builds. Tested with hs-teir1, hs-tier2, jdk-tier1 and 
>>> jdk-tier2. The hs-tier3, hs-tier4 and hs-tier4 are in progress.
>>>
>>> Thanks,
>>> Jiangli
>>>
>



More information about the build-dev mailing list