RFR (S): 8214277: Use merged G1ArchiveRegionMap for open and closed archive heap regions

Thomas Schatzl thomas.schatzl at oracle.com
Thu Jan 9 11:37:54 UTC 2020


Hi Kim, Jiangli,

   thanks for your reviews.

On 09.01.20 01:25, Kim Barrett wrote:
>> On Jan 8, 2020, at 8:38 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
>>
>> Hi all,
>>
>>   could I have reviews for this small cleanup/simplification that merges the open and closed archive region map into a single one?
>>
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8214277
>> Webrev:
>> http://cr.openjdk.java.net/~tschatzl/8214277/webrev/
>> Testing:
>> hs-tier1-5 (almost done, no issues), local gc/g1 jtreg
>>
>> Thanks,
>>   Thomas
> 
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/g1/g1Allocator.inline.hpp
>   151 inline void G1ArchiveAllocator::clear_range_archive(MemRegion range, bool open) {
> 
> clear_range_archive no longer uses the open argument for anything
> other than logging.  Is it worth keeping?

Removed.

> 
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/g1/g1Allocator.inline.hpp
>   186   return (archive_check_enabled() &&
>   187          (_archive_region_map.get_by_address((HeapWord*)object) != G1ArchiveRegionMap::NoArchive));
> 
> The indentation of line 187 is confusing; the indentation suggests the
> open paren there is at the same nesting level as the one on the
> preceeding line directly above.  The first open paren on line 186 and
> the associated close could just be removed.
> 
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/g1/g1Allocator.inline.hpp
>   162   // This is the out-of-line part of is_closed_archive_object test, done separately
>   163   // to avoid additional performance impact when the check is not enabled.
> 
> Pre-existing: Given that it is in an inline definition, the accuracy
> of this comment seems questionable.

Removed.

> 
> ------------------------------------------------------------------------------
> 
> Jiangli already pointed out that _open_archive_region_map is no longer
> used.
> 

All fixed in
http://cr.openjdk.java.net/~tschatzl/8214277/webrev.0_to_1 (diff)
http://cr.openjdk.java.net/~tschatzl/8214277/webrev.1/ (full)

Thanks,
   Thomas



More information about the hotspot-gc-dev mailing list