RFR: 8207812: Implement Dynamic CDS Archive
Karen Kinnear
karen.kinnear at oracle.com
Thu Apr 25 16:08:34 UTC 2019
Ioi and Calvin,
Thank you for the thoughtful responses. I totally agree that keeping this in mind as part of where
we are going longer term is the better approach.
And I do like the idea of giving customers suggestions for their scripts - whether that
is someone’s blog or ...
thanks,
Karen
> On Apr 23, 2019, at 4:43 PM, Ioi Lam <ioi.lam at oracle.com> wrote:
>
>
>
> 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> <mailto: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> <mailto: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>>
>>>> >> <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/~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/>>
>>>> >>
>>>> <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