RFR: 8207812: Implement Dynamic CDS Archive
Ioi Lam
ioi.lam at oracle.com
Tue Apr 23 20:43:42 UTC 2019
On 4/23/19 1:26 PM, Calvin Cheung wrote:
>
>
> On 4/23/19, 11:16 AM, Karen Kinnear wrote:
>> Calvin,
>>
>> I added to the CSR a comment from my favorite customer - relative to
>> the user model for the command-line flags.
>> He likes the proposal to reduce the number of steps a customer has to
>> perform to get startup and footprint benefits
>> from the archive.
>>
>> The comment was that it would be very helpful if the user only needed
>> to change their scripts once - so
>> a single command-line argument would create a dynamic archive if one
>> did not exist, and use it if it
>> already existed.
>>
>> Is there a way to evolve the ArchiveClassesAtExit=<dynamic archive>
>> to have that functionality?
> One drawback of this proposal is that the ArchiveClassesAtExit option
> has 2 meanings which I find confusing.
> Maybe eventually we can do some kind of automatic CDS archive dumping
> without having to specify any command line option. Such as when a java
> app is run at the first time, there will be some CDS archive created
> with a unique name. Subsequent run of the same app will make use of
> the archive.
When we scoped this JEP, we wanted to provide just the minimal building
blocks, so a user could implement automation on top of the JVM.
Something like
ARCHIVE=foo.jsa
if test -f $ARCHIVE; then
FLAG="-XX:SharedArchiveFile=$ARCHIVE"
else
FLAG="-XX:ArchiveClassesAtExit=$ARCHIVE"
fi
$JAVA_HOME/bin/java -cp foo.jar $FLAG FooApp
Note that you also need to update the archive if the Java version has
changed, so the test would be a little more complicated
ARCHIVE=foo.jsa
VERSION=foo.version
if test -f $ARCHIVE -a -f $VERSION && cmp -s $VERSION
$JAVA_HOME/release; then
FLAG="-XX:SharedArchiveFile=$ARCHIVE"
else
FLAG="-XX:ArchiveClassesAtExit=$ARCHIVE"
cp -f $JAVA_HOME/release $VERSION
fi
$JAVA_HOME/bin/java -cp foo.jar $FLAG FooApp
As Calvin mentioned, we are planning to make the archive management more
automatic. So eventually you might be able to do something like
java -Xshare:reallyauto -cp foo.jar FooApp
And the JSA file will be automatically generated if necessary. We
probably need some logic to delete older archives to avoid filling up
the disk.
I think the automation feature needs to be carefully planned out, so we
should do that in a follow-up RFE or JEP.
Thanks
- Ioi
>>
>> thanks,
>> Karen
>>
>> p.s. I think it makes more sense to put performance numbers in the
>> implementation RFE comments rather than the JEP
>> comments
> We found a bug yesterday and will run more performance tests. I'll put
> the performance numberss in the implementation RFE.
>
> thanks,
> Calvin
>>
>>> On Apr 22, 2019, at 5:16 PM, Jiangli Zhou <jianglizhou at google.com
>>> <mailto:jianglizhou at google.com>> wrote:
>>>
>>> Hi Calvin,
>>>
>>> Can you please also publish the final performance numbers in the JEP
>>> 350 (or the implementation RFE) comment section?
>>>
>>> Thanks,
>>> Jiangli
>>>
>>> On Mon, Apr 22, 2019 at 10:07 AM Calvin Cheung
>>> <calvin.cheung at oracle.com <mailto:calvin.cheung at oracle.com>> wrote:
>>>
>>> Hi Karen,
>>>
>>> Thanks for your review!
>>> Please see my replies in-line below.
>>>
>>> On 4/19/19, 9:29 AM, Karen Kinnear wrote:
>>> > Calvin,
>>> >
>>> > Many thanks for all the work getting this ready, significantly
>>> > enhancing the testing and bug fixes.
>>> >
>>> > I marked the CSR as reviewed-by - it looks great!
>>> >
>>> > I reviewed this set of changes - I did not review the tests - I
>>> assume
>>> > you can get someone
>>> > else to do that. I am grateful that Jiangli and Ioi are going to
>>> > review this also - they are much closer to
>>> > the details than I am.
>>> >
>>> > 1. Do you have any performance numbers?
>>> > 1a. Startup: does using a combined dynamic CDS archive + base
>>> archive
>>> > give similar startup benefits
>>> > when you have the same classes in the archives?
>>> Below are some performance numbers from Eric, each number is for
>>> 50 runs:
>>> (base: using the default CDS archive,
>>> test: using the dynamic archive,
>>> Eric will get some numbers with a single archive which I think
>>> that's
>>> what you're looking for)
>>>
>>> Lambda-noop:
>>> base:
>>> 0.066441427 seconds time elapsed
>>> test:
>>> 0.075428824 seconds time elapsed
>>>
>>> Noop:
>>> base:
>>> 0.057614537 seconds time elapsed
>>> test:
>>> 0.066061557 seconds time elapsed
>>>
>>> Netty:
>>> base:
>>> 0.827013307 seconds time elapsed
>>> test:
>>> 0.604982805 seconds time elapsed
>>>
>>> Spring:
>>> base:
>>> 2.376707358 seconds time elapsed
>>> test:
>>> 1.927618893 seconds time elapsed
>>>
>>> The first 2 apps only have 2 to 3 classes in the dynamic archive.
>>> So the
>>> overhead is likely due to having to open and map the dynamic
>>> archive and
>>> performs checking on header, etc. For small apps, I think it's
>>> better to
>>> use a single archive. The Netty app has around 1400 classes in the
>>> dynamic archive; the Spring app has about 3700 classes in the
>>> dynamic
>>> archive.
>>>
>>> I also used our LotsOfClasses test to collect some perf numbers.
>>> This is
>>> more like runtime performance, not startup performance.
>>>
>>> With dynamic archive (100 runs each):
>>> real 2m37.191s
>>> real 2m36.003s
>>> Total loaded classes = 24254
>>> Loaded from base archive = 1186
>>> Loaded from top archive = 23042
>>> Loaded from jrt:/ (runtime module) = 26
>>>
>>> With single archive (100 runs each):
>>> real 2m38.346s
>>> real 2m36.947s
>>> Total loaded classes = 24254
>>> Loaded from archive = 24228
>>> Loaded from jrt:/ (runtime module) = 26
>>>
>>> >
>>> > 1b. Do you have samples of uses of the combined dynamic CDS
>>> archive +
>>> > base archive vs. a single
>>> > static archive built for an application?
>>> > - how do the sets of archived classes differ?
>>> Currently, the default CDS archive contains around 1187 classes.
>>> With
>>> the -XX:ArchiveClassesAtExit option, if the classes are not found
>>> in the
>>> default CDS archive, they will be archived in the dynamic
>>> archive. The
>>> above LotsOfClasses example shows some distributions between
>>> various
>>> archives.
>>> > - one note was that the AtExit approach exclude list adds
>>> anything
>>> > that has not yet linked - does that make a significant
>>> difference in
>>> > the number of classes that are archived? Does that make a
>>> difference
>>> > in either startup time or in application execution time? I
>>> could see
>>> > that going either way.
>>> As the above numbers indicated, there's not much difference in
>>> terms of
>>> execution time using a dynamic vs a single archive with a large
>>> number
>>> of classes loaded. The numbers from Netty and Spring apps show an
>>> improvement over default CDS archive.
>>> >
>>> > 1c. Any sense of performance cost for first run - how much time
>>> does
>>> > it take to create an incremental archive?
>>> > - is the time comparable to an existing dump for a single
>>> archive
>>> > for the application?
>>> > - this is an ease-of-use feature - so we are not expecting
>>> that to
>>> > be fast
>>> > - the point is to set expectations in our documentation
>>> I did some rough measurements with the LotsOfClasses test with
>>> around
>>> 15000 classes in the classlist.
>>>
>>> Dynamic archive dumping (one run each):
>>> real 0m19.756s
>>> real 0m20.241s
>>>
>>> Static archive dumping (one run each):
>>> real 0m17.725s
>>> real 0m16.993s
>>> >
>>> > 2. Footprint
>>> > With two archives rather than one, is there a significant
>>> footprint
>>> > difference? Obviously this will vary by app and archive.
>>> > Once again, the point is to set expectations.
>>> Sizes of the archives for the LotsOfClasses test in 1a.
>>>
>>> Single archive: 242962432
>>> Default CDS archive: 12365824
>>> Dynamic archive: 197525504
>>>
>>> >
>>> > 3. Runtime performance
>>> > With two sets of archived dictionaries & symbolTables - is
>>> there any
>>> > significant performance cost to larger benchmarks, e.g. for class
>>> > loading lookup for classes that are not in the archives? Or
>>> symbol
>>> > lookup?
>>> I used the LotsOfClasses test again. This time archiving about
>>> half of
>>> the classes which will be loaded during runtime.
>>>
>>> Dynamic archive (10 runs each):
>>> real 0m30.214s
>>> real 0m29.633s
>>> Loaded classes = 24254
>>> Loaded from dynamic archive: 13168
>>>
>>> Single archive (10 runs each):
>>> real 0m32.383s
>>> real 0m32.905s
>>> Loaded classes = 24254
>>> Loaded from single archive = 15063
>>> >
>>> > 4. Platform support
>>> > Which platforms is this supported on?
>>> > Which ones did you test? For example, did you run the tests on
>>> Windows?
>>> I ran the jtreg tests via mach5 on all 4 platforms (Linux, Mac,
>>> Solaris,
>>> Windows).
>>> >
>>> > Detailed feedback on the code: Just minor comments - I don’t
>>> need to
>>> > see an updated webrev:
>>> I'm going to look into your detailed feedback below and may reply
>>> in a
>>> separate email.
>>>
>>> thanks,
>>> Calvin
>>> >
>>> > 1. metaSpaceShared.hpp
>>> > line 156:
>>> > what is the hardcoded -100 for? Should that be an enum?
>>> >
>>> > 2. jfrRecorder.cpp
>>> > So JFR recordings are disabled if DynamicDumpSharedSpaces?
>>> > why?
>>> > Is that a future rfe?
>>> >
>>> > 3. systemDictionaryShared.cpp
>>> > Could you possibly add a comment to add_verification_constraint
>>> > for if (DynamicDumpSharedSpaces)
>>> > return false
>>> >
>>> > -- I think the logic is:
>>> > because we have successfully linked any instanceKlass we
>>> archive
>>> > with DynamicDumpSharedSpaces, we have resolved all the
>>> constraint classes.
>>> >
>>> > -- I didn't check the order - is this called before or after
>>> > excluding? If after, then would it make sense to add an assertion
>>> > here is_linked? Then if you ever change how/when linking is
>>> done, this
>>> > might catch future errors.
>>> >
>>> > 4. systemDictionaryShared.cpp
>>> > EstimateSizeForArchive::do_entry
>>> > Is it the case that for info.is_builtin() there are no
>>> verification
>>> > constraints? So you could skip that calculation? Or did I
>>> misunderstand?
>>> >
>>> > 5. compactHashtable.cpp
>>> > serialize/header/calculate_header_size
>>> > -- could you dynamically determine size_of header so you don't
>>> need
>>> > to hardcode a 5?
>>> >
>>> > 6. classLoader.cpp
>>> > line 1337: //FIXME: DynamicDumpSharedSpaces and
>>> --patch-modules are
>>> > mutually exclusive.
>>> > Can you clarify for me:
>>> > My memory of the base archive is that we do not allow the
>>> following
>>> > options at dump time - and these
>>> > are the same for the dynamic archive: —limit-modules,
>>> > —upgrade-module-path, —patch-module.
>>> >
>>> > I have forgotten:
>>> > Today with UseSharedSpaces - do we allow these flags? Is that
>>> also the
>>> > same behavior with the dynamic
>>> > archive?
>>> >
>>> > 7. classLoaderExt.cpp
>>> > assert line 66: only used with -Xshare:dump
>>> > -> "only used at dump time"
>>> >
>>> > 8. symbolTable.cpp
>>> > line 473: comment // used by UseSharedArchived2
>>> > — command-line arg name has changed
>>> >
>>> > 9. filemap.cpp
>>> > Comment lines 529 ...
>>> > Is this true - that you can only support dynamic dumping with the
>>> > default CDS archive? Could you clarify what the restrictions are?
>>> > The CSR implies you can support “a specific base CDS archive"
>>> > - so base layer can not have appended boot class path
>>> > - and base layer can't have a module path
>>> >
>>> > What can you specify for the dynamic dumping relative to the
>>> base archive?
>>> > - matching class path?
>>> > - appended class path?
>>> > in future - could it have a module path that matched the base
>>> archive?
>>> >
>>> > Should any of these restrictions be clarified in
>>> documentation/CSR
>>> > since they appear to be new?
>>> >
>>> > 10. filemap.cpp
>>> > check_archive
>>> > Do some of the return false paths skip performing os::close(fd)?
>>> >
>>> > and get_base_archive_name_from_header
>>> > Does the first return false path fail to os::free(dynamic_header)
>>> >
>>> > lines 753-754: two FIXME comments
>>> >
>>> > Could you delete commented out line 1087 in filemap.cpp ?
>>> >
>>> > 11. filemap.hpp
>>> > line 214: TODO left in
>>> >
>>> > 12. metaspace.cpp
>>> > line 1418 FIXME left in
>>> >
>>> > 13. java.cpp
>>> > FIXME: is this the right place?
>>> > For starting the DynamicArchive::dump
>>> >
>>> > Please check with David Holmes on that one
>>> >
>>> > 14. dynamicArchive.hpp
>>> > line 55 (and others): MetsapceObj -> MetaspaceObj
>>> >
>>> > 15. dynamicArchive.cpp
>>> > line 285 rel-ayout -> re-layout
>>> >
>>> > lines 277 && 412
>>> > Do we archive array klasses in the base archive but not in the
>>> dynamic
>>> > archive?
>>> > Is that a potential RFE?
>>> > Is it possible that GatherKlassesAndSymbols::do_unique_ref
>>> could be
>>> > called with an array class?
>>> > Same question for copy_impl?
>>> >
>>> > line 934: "no onger" -> "no longer"
>>> >
>>> > 16. What is AllowArchivingWithJavaAgent? Is that a hook for a
>>> > potential future rfe?
>>> > Do you want to check in that code at this time? In product?
>>> >
>>> > thanks,
>>> > Karen
>>> >
>>> >
>>> >> On Apr 11, 2019, at 5:18 PM, Calvin Cheung
>>> <calvin.cheung at oracle.com <mailto:calvin.cheung at oracle.com>
>>> >> <mailto:calvin.cheung at oracle.com
>>> <mailto:calvin.cheung at oracle.com>>> wrote:
>>> >>
>>> >> This is a follow-up on the preliminary code review sent by
>>> Jiangli in
>>> >> January[1].
>>> >>
>>> >> Highlights of changes since then:
>>> >> 1. New vm option for dumping a dynamic archive
>>> >> (-XX:ArchiveClassesAtExit=<dynamic archive>) and enhancement
>>> to the
>>> >> existing -XX:SharedArchiveFile option. Please refer to the
>>> >> corresponding CSR[2] for details.
>>> >> 2. New way to run existing AppCDS tests in dynamic CDS archive
>>> mode.
>>> >> At the jtreg command line, the user can run many existing AppCDS
>>> >> tests in dynamic CDS archive mode by specifying the following:
>>> >> -vmoptions:-Dtest.dynamic.cds.archive=true
>>> >> <jdk>/open/test/hotspot/jtreg:hotspot_appcds_dynamic
>>> >> We will have a follow-up RFE to determine in which tier the
>>> above
>>> >> tests should be run.
>>> >> 3. Added more tests.
>>> >> 4. Various bug fixes to improve stability.
>>> >>
>>> >> RFE:https://bugs.openjdk.java.net/browse/JDK-8207812
>>> >> webrev:
>>> >>http://cr.openjdk.java.net/~ccheung/8207812_dynamic_cds_archive/webrev.00/
>>> <http://cr.openjdk.java.net/%7Eccheung/8207812_dynamic_cds_archive/webrev.00/>
>>> >>
>>> <http://cr.openjdk.java.net/%7Eccheung/8207812_dynamic_cds_archive/webrev.00/>
>>> >>
>>> >> (The webrev is based on top of the following rev:
>>> >>http://hg.openjdk.java.net/jdk/jdk/rev/805584336738)
>>> >>
>>> >> Testing:
>>> >> - mach5 tiers 1- 3 (including the new tests)
>>> >> - AppCDS tests in dynamic CDS archive mode on linux-x64 (a
>>> few
>>> >> tests require more investigation)
>>> >>
>>> >> thanks,
>>> >> Calvin
>>> >>
>>> >> [1]
>>> >>https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2019-January/032176.html
>>> >> [2]https://bugs.openjdk.java.net/browse/JDK-8221706
>>> >
>>>
>>
More information about the hotspot-runtime-dev
mailing list