RFR: 8261455: Automatically generate the CDS archive if necessary [v3]

Calvin Cheung ccheung at openjdk.java.net
Fri Oct 22 22:38:10 UTC 2021


On Thu, 21 Oct 2021 23:55:33 GMT, Yumin Qi <minqi at openjdk.org> wrote:

>> Please review,
>>   When shared archive (dynamic archive) failed to map due to damage of the archive file, dump/run jdk version mismatch or non-existence file etc, the new patch will automatically create a new shared archive if -XX:+AutoCreateSharedArchive specified with the name based on SharedArchiveFile.
>>   This is a revised patch based on the old PR: 5077 and after bug 8273152 integrated.
>> 
>> Tests: tier1,tier2,tier3,tier4
>> 
>> Thanks
>> Yumin
>
> Yumin Qi has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Make FileHeaderHelper::read_base_archive_name return char*, also cleaned up some check code

Looks good overall. Just a few minor comments.

src/hotspot/share/runtime/java.cpp line 60:

> 58: #include "oops/symbol.hpp"
> 59: #include "prims/jvmtiExport.hpp"
> 60: #include "runtime/arguments.hpp"

Is this `#include` needed?

test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/TestAutoCreateSharedArchive.java line 153:

> 151:         // 10.3 run with the created dynamic archive with -XX:+AutoCreateSharedArchive should pass
> 152:         print("10.3 run with the created dynamic archive with -XX:+AutoCreateSharedArchive should pass");
> 153:         run(TOP_NAME,

Maybe add a check to make sure the dynamic archive hasn't been changed after run?

test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/TestAutoCreateSharedArchive.java line 255:

> 253: 
> 254:          // 20 Testing with -XX:SharedArchiveFile=top:base
> 255:          print("20 Testing with -XX:SharedArchiveFile=top:base");

Do you mean `-XX:SharedArchiveFile=base:top`?

test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/TestAutoCreateSharedArchive.java line 273:

> 271:                        .shouldContain("Dumping shared data to file:")
> 272:                        .shouldContain(TOP_NAME);
> 273:              });

Suggestion: add `checkFileExists(TOP_NAME)`.

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

Changes requested by ccheung (Reviewer).

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


More information about the hotspot-runtime-dev mailing list