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

Thomas Stüfe thomas.stuefe at gmail.com
Wed Aug 7 07:26:41 UTC 2019


Hi Erik,

This looks good to me. I think it may also be more efficient since you have
better locality. I have no strong feelings wrt using a union.

Cheers, Thomas.

On Wed 7. Aug 2019 at 08:54, Erik Österlund <erik.osterlund at oracle.com>
wrote:

> 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
> >
>


More information about the hotspot-runtime-dev mailing list