On Mon, 11 Apr 2022 11:55:33 GMT, Roman Kennke <rken...@openjdk.org> wrote:

>> JVMTI heap walking marks objects in order to track which have been visited 
>> already. In order to do that, it uses bits in the object header. Those are 
>> the same bits that are also used by some GCs to mark objects (the lowest two 
>> bits, also used by locking code). Some GCs also use the bits in order to 
>> indicate 'forwarded' objects, where the upper bits of the header represent 
>> the forward-pointer. In the case of Shenandoah, it's even more problematic 
>> because this happens concurrently, even while JVMTI heap walks can 
>> intercept. So far we carefully worked around that problem, but it becomes 
>> very problematic in Lilliput, where accesses to the Klass* also requires to 
>> decode the header, and figure out what bits means what.
>> 
>> In addition to that, marking objects in their header requires that the 
>> original header gets saved and restored. We only do that for 'interesting' 
>> headers, that is headers that have a stack-lock, monitor or hash-code. All 
>> other headers are reset to their default value. This means we are losing 
>> object's GC age. This is not catastrophic, but nontheless interferes with 
>> GC. 
>> 
>> JFR already has a datastructure called BitSet to support object marking 
>> without messing with object's headers. We can use that in JVMTI too.
>> 
>> Testing:
>>  - [x] tier1
>>  - [x] tier2
>>  - [x] tier3
>>  - [x] serviceability/jvmti
>>  - [x] vmTestbase/nsk/jvmti
>
> Roman Kennke has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add some basic tests for ObjectBitSet

I think this looks great and thank you for all your work resolving comments and 
bringing this in from the lilliput changes you were going to make. This is a 
big improvement imo and lets us potentially fix other problems in the jvmti tag 
map implementation. The template instantiation doesn't bother me. There's only 
2 and it's not a big code bloat and it's the way the code has been dealing with 
memflags. Maybe the mechanism should be revisited, like the GC change that Kim 
pointed to, but I don't see the motivation to make it something different.  
Maybe a later RFE can propose a different mechanism for memflags in general.  
We picked this because we thought it was simple.
Thank you for your work on this, Roman and your comments Thomas.

test/hotspot/gtest/utilities/test_objectBitSet.cpp line 38:

> 36: // allocate a fragement for the memory range starting at 0 and mark the 
> first bit when passing NULL.
> 37: // In the absense of any error handling, I am not sure what would 
> possibly be a reasonable better
> 38: // way to do it, though.

Thanks for the comment. If it matters someday we'll have the comment..

-------------

Marked as reviewed by coleenp (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7964

Reply via email to