RFR(M) 8250990 Consolidate object copying code for CDS static/dynamic archive dumping
Coleen Phillimore
coleen.phillimore at oracle.com
Tue Aug 4 12:36:08 UTC 2020
Ioi, This version looks better. I still think there's too many internal details that are exposed in archiveBuilder.hpp, but this can be cleaned up in another pass.
Can you fix the typo: _estimated_metsapceobj_bytes
Otherwise, this is reviewed.
Coleen
----- Original Message -----
From: ioi.lam at oracle.com
To: hotspot-runtime-dev at openjdk.java.net
Sent: Tuesday, August 4, 2020 1:59:27 AM GMT -05:00 US/Canada Eastern
Subject: Re: RFR(M) 8250990 Consolidate object copying code for CDS static/dynamic archive dumping
Here's an updated version after offline discussion with Coleen:
http://cr.openjdk.java.net/~iklam/jdk16/8250990-consolidate-static-dynamic-cds-objcopy.v02/
http://cr.openjdk.java.net/~iklam/jdk16/8250990-consolidate-static-dynamic-cds-objcopy.v02-delta/
- Moved DumpAllocStats into its own cpp/hpp files
- Removed the use of inline.hpp files from archiveBuilder.hpp
- Changed comments about the old ArchiveCompactor class to
the new AcrchiveBuilder class.
- Removed CDSAddSymbol_lock which is no longer used.
Thanks
- Ioi
On 8/3/20 3:11 PM, Ioi Lam wrote:
> There turns out to be a lot of duplication in the static/dynamic
> dumping code. To keep the changes manageable, I've reorganized the REF
> into a few sub-tasks. This particular review is for:
>
> https://bugs.openjdk.java.net/browse/JDK-8250990
> http://cr.openjdk.java.net/~iklam/jdk16/8250990-consolidate-static-dynamic-cds-objcopy.v01/
>
> https://wiki.openjdk.java.net/display/HotSpot/How+CDS+Copies+Class+Metadata+into+the+Archive
>
>
> I've also updated the e-mail subject to use the new JBS ID (JDK-8250990).
>
> This patch is an intermediate step. Some code might look like they are
> in the wrong place -- for example: _estimated_metsapceobj_bytes has
> been moved to archiveBuilder.hpp, but _estimated_hashtable_bytes is
> still in dynamicArchive.cpp. The latter will eventually be moved as
> part of the second step (JDK-8250989
> Consolidate buffer allocation code for CDS static/dynamic archive
> dumping).
>
> (Coleen has given me feedback off-line. I'll post a new version soon
> to limit the API in archiveBuilder.hpp)
>
> Thanks
> - Ioi
>
> On 7/31/20 11:36 PM, Ioi Lam wrote:
>>
>> Currently there are two very similar implementations for static CDS
>> archive
>> dumping (metaspaceShared.cpp) and dynamic CDS archive dumping
>> (dynamicArchive.cpp).
>>
>> I've merged the two versions and moved to a new file,
>> archiveBuilder.cpp.
>>
>> I also improved the copying algorithm to speed up dumping. Times for
>> dumping
>> 20000+ classes:
>>
>> static dynamic
>> Old 42.655 sec 67.014 sec
>> New 37.027 sec 34.974 sec
>>
>> Normally I would not try to optimize when refactoring code. I made an
>> earlier, naive attempt to use the old dynamic dumper for static dumping
>> (because dynamic dumping is essentially a superset of static dumping).
>> However, that caused significant regression in static dumping speed.
>>
>> Hopefully the new algorithm is both faster and easier to understand.
>> Please see the wiki document for more information.
>>
>> Tested with mach5 tiers 1-4.
>>
>> Thanks
>> - Ioi
>
More information about the hotspot-runtime-dev
mailing list