RFR: 8229027: Improve how JNIHandleBlock::oops_do distinguishes oops from non-oops

[Erik Österlund]

Hi Dean,

Thanks for having a look at this code.

Yes indeed. I actually thought about using union, but decided not to in 
the end. Here is why: if sizeof(oop) != sizeof(uintptr_t), then we would 
already be in a world of trouble as there is already so much code that 
relies on that, which would fundamentally break, that I think it's fair 
to assume such a change is very unlikely to happen.

Here are a few examples of things that would be unfortunate if 
sizeof(oop) != sizeof(uintptr_t):

* All GCs use OopClosures, requiring oops in Java objects in the heap to 
be oops. But we need to be able to use Atomic on oops. So they can't be 
larger than uintptr_t.
* Having a different object layout in debug and release would be very 
unfortunate in general, because we would be testing something different 
to what a release build looks like.
* The JIT and a lot of platform dependent hand written assembly code 
would have to change to deal with reading wide oops from the heap, and 
all hardcoded assumptions everywhere else in e.g. compilers that oops 
are pointers would probably not work.

However, having said that, I agree that it would be a good idea to at 
least make the assumption made more clear. I inserted a STATIC_ASSERT 
that checks the sizes, with an appropriate comment explaining that if 
you are changing sizeof(oop), then you have to change this freelist 
handling. At least this way, if someone eventually wants to change this 
assumption, they would notice this very explicitly as you could not 
compile without knowing this code must be changed.

What do you think? If you still think using union is better, then I am 
open to doing that as well, and do not have strong opinions about it.

Incremental:
http://cr.openjdk.java.net/~eosterlund/8229027/webrev.00_01/

Full:
http://cr.openjdk.java.net/~eosterlund/8229027/webrev.01/

Thanks,
/Erik

On 2019-08-07 07:30, dean.long at oracle.com wrote:
> Hi Erik.  Doesn't this only work if sizeof (class oop) <= sizeof 
> (uintptr_t)?  If a compiler uses more space for class oop, or we add a 
> debug field, then it will break.  Wouldn't a union be safer?
> 
> dl
> 
> On 8/6/19 6:04 AM, Erik Österlund wrote:
>> Hi,
>>
>> In JNIHandleBlock::oops_do(), the blocks may contain free list next 
>> pointers or oops. The distinction is detected by asking the 
>> CollectedHeap if the pointer value is in the heap reserve or not.
>> This forces GCs to have one single contiguous heap reservation, 
>> without holes in it, which seems like an unnecessary restriction (and 
>> effectively blocks a ZGC mac port).
>>
>> This patch removes this reliance by tagging the free list next 
>> pointers instead, allowing the distinction between oops and freelist 
>> pointers based on the low order bit value.
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8229027
>>
>> Webrev:
>> http://cr.openjdk.java.net/~eosterlund/8229027/webrev.00/
>>
>>
>> Thanks,
>> /Erik
>