RFR: 8193213 & 8182731: Make the UseAppCDS option obsolete
Magnus Ihse Bursie
magnus.ihse.bursie at oracle.com
Wed Apr 25 09:17:36 UTC 2018
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?
Build changes look good.
> 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 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.
> makeCommandLineForAppCDS should just be removed (yes that will cause
> some churn in the other tests) - but this can be a follow up test
> 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
>> 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
>> - 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.
More information about the hotspot-runtime-dev