RFR: 8224815: Remove non-GC uses of CollectedHeap::is_in_reserved()
Erik Österlund
erik.osterlund at oracle.com
Thu Aug 29 13:20:40 UTC 2019
Hi Stefan,
Thanks for having an in-depth look at this patch.
On 2019-08-29 15:07, Stefan Karlsson wrote:
> Hi Erik,
>
> On 2019-08-09 09:43, Erik Österlund wrote:
>> Hi,
>>
>> The CollectedHeap::is_in_reserved() function assumes the GC has a
>> contiguous heap. This might not be the case, and hence the shared code
>> should not make assumptions about that. It should use
>> CollectedHeap::is_in() instead.
>>
>> However, CompressedOops inherently assumes the heap is contiguous, so
>> I let it know about the reserved region.
>>
>> This patch depends on 8229278 and 8229189, which are both out for
>> review right now.
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8224815
>>
>> Webrev:
>> http://cr.openjdk.java.net/~eosterlund/8224815/webrev.00/
>
>
> I think this looks good, but have a few cleanups that I'd like to suggest:
>
> http://cr.openjdk.java.net/~stefank/8224815/webrev.01.review.00/
>
> 1) Changed some parameters to accept void* to get rid of some casts
>
> 2) Replaced address with HeapWord* in GC code.
>
> 3) Moved the code that was moved to initialize_reserved_region and moved
> it into Universe::reserve_heap. I could therefore restore the location
> of the initialize_reserved_region calls. Fixed a test that relied on the
> logging order.
>
> 4) Moved the heap address range MemRegion out of NarrowPtrStruct into
> CompressedOops
>
> 5) Replaced some DEBUG_ONLY with NOT_DEBUG_RETURN.
>
> 6) Minor cleanups
>
> I ran this through tier1.
Thank you for the bits. I think your improvements did make the code
nicer, and I agree with all suggestions.
> It's a bit annoying that we still have CollectedHeap::_reserved, but I
> understand why you left it for now. However, I see that the
> Serviceability Agent uses that field. Did you think about if that
> affects ZGC support in SA in any way?
Yeah it annoyed me a bit too. And no I haven't looked into what the SA
does differently if you don't specify a reserve region. I will try to
look into that.
Thanks,
/Erik
> Thanks,
> StefanK
>
>>
>> Thanks,
>> /Erik
More information about the hotspot-dev
mailing list