RFR: 8224815: Remove non-GC uses of CollectedHeap::is_in_reserved()

[Erik Österlund]

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