RFR 8042668: GC Support for shared heap ranges in CDS (RE: JDK-8059092)
Jiangli Zhou
jiangli.zhou at oracle.com
Mon Jun 1 17:47:37 UTC 2015
Hi Tom and Per,
On Jun 1, 2015, at 8:22 AM, Tom Benson <tom.benson at oracle.com> wrote:
> Hi Per,
> Thanks very much for the review.
>
> On 6/1/2015 10:35 AM, Per Liden wrote:
>> Hi Tom,
>>
>> On 2015-05-29 23:30, Tom Benson wrote:
>>> Hi,
>>> Please review these changes for JDK-8042668, which constitute the GC
>>> support for JDK-8059092 for storing interned strings in CDS archives
>>> (JEP 250). The RFR for JDK-8059092 was recently posted by Jiangli Zhou,
>>> and it would be best if overall comments could go to that thread, with
>>> GC-specific comments here.
>>>
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8042668
>>> Webrev: http://cr.openjdk.java.net/~brutisso/8042668/webrev.00/
>>
>>
>> Maybe it's just me, but the concept of "recording" feels a bit strange in this context. May I suggest that we remove the "record" and "recording" part of the names and instead just call it an "archive" that we can allocate in? Something like:
>>
>> class G1ArchiveAllocator ... {
>> HeapWord* mem_allocate(...);
>>
>> void finalize(...);
>>
>> ...
>> };
>>
>> class G1CollectedHeap ... {
>> void begin_archive_mem_allocate();
>>
>> bool is_archive_mem_allocation_too_large(...);
>>
>> HeapWord* archive_mem_allocate(...);
>>
>> void end_archive_mem_allocate(...);
>>
>> ...
>> };
>
> Hmmm.... Yes, I guess the name "RecordingAllocator" does show the evolution of the design, more than the ultimate use. It was named "recording" because it allowed a way to keep track of the recorded ranges, in contrast with an earlier design that allocated a block of memory up front. I'm fine with changing this to an ArchiveAllocator as you suggest, if I hear no objections.
Removing the “Recording” part sounds good to me too.
Thanks,
Jiangli
>
>>
>>
>> g1CollectedHeap.cpp
>> -------------------
>>
>> * In G1CollectedHeap::end_record_alloc_range(), shouldn't we delete the allocator as the last step?
>>
>
> Yes. I think I made that change at one point and then removed it for some reason, which may be gone. I'll re-make it.
>
>
>> * I guess this change could be skipped, as it makes the comment slightly malformed.
>>
>> - // We ignore humongous regions, we left the humongous set unchanged
>> + // We ignore humongous regions.
>> + // We left the humongous set unchanged,
>>
>
> OK.
>
>>
>> g1Allocator.hpp
>> ---------------
>>
>> + class G1RecordingAllocator : public CHeapObj<mtGC> {
>> + friend class VMStructs;
>>
>> You could skip this friend declaration, since it's not accessed by VMStructs. Only needed if the class is exposed in the SA.
>>
>>
>
> OK. Thanks,
> Tom
>
>> cheers,
>> /Per
>>
>>>
>>> These changes add a new "archive" region type to G1. The description
>>> field in JDK-8042668 contains an "Implementation Notes" section which
>>> describes components of the design, and should be useful for a code
>>> review. The overview:
>>>
>>> "Archive" regions are G1 regions that are not modifiable by GC,
>>> being neither scavenged nor compacted, or even marked in the object
>>> header. They can contain no pointers to non-archive heap regions,
>>> and object headers point to shared CDS metaspace (though this last
>>> point is not enforced by G1). Thus, they allow the underlying
>>> hardware pages to be shared among multiple JVM instances.
>>>
>>> In short, a dump-time run (using -Xshare:dump) will allocate space
>>> in the Java heap for the strings which are to be shared, copy the
>>> string objects and arrays to that space, and then archive the entire
>>> address range in the CDS archive. At restore-time (using
>>> -Xshare:on), that same heap range will be allocated at JVM init
>>> time, and the archived data will be mmap'ed into it. GC must treat
>>> the range as 'pinned,' never moving or writing to any objects within
>>> it, so that cross-JVM sharing will work.
>>>
>>> Testing: All testing for JDK-8059092 included this code. Manual
>>> testing with prototype calls to the new GC support was performed before
>>> integration, along with JPRT and benchmark runs.
>>>
>>> Performance: The GC changes had no significant impact on SpecJBB, JVM,
>>> or Dacapo benchmarks, run on x64 Linux. However, a small (~1%) increase
>>> in Full GC times was seen in tests when the shared string support was
>>> not in use, when runs are configured to encounter them. When shared
>>> strings ARE in use, the impact could be as high as 5% for a likely
>>> worst-case. Please see the JBS entry for a discussion.
>>>
>>> Thanks,
>>> Tom
>>>
>
More information about the hotspot-gc-dev
mailing list