On Wed, 4 Nov 2020 00:08:10 GMT, Coleen Phillimore <cole...@openjdk.org> wrote:

>> This change turns the HashTable that JVMTI uses for object tagging into a 
>> regular Hotspot hashtable - the one in hashtable.hpp with resizing and 
>> rehashing.   Instead of pointing directly to oops so that GC has to walk the 
>> table to follow oops and then to rehash the table, this table points to 
>> WeakHandle.  GC walks the backing OopStorages concurrently.
>> 
>> The hash function for the table is a hash of the lower 32 bits of the 
>> address.  A flag is set during GC (gc_notification if in a safepoint, and 
>> through a call to JvmtiTagMap::needs_processing()) so that the table is 
>> rehashed at the next use.
>> 
>> The gc_notification mechanism of weak oop processing is used to notify Jvmti 
>> to post ObjectFree events.  In concurrent GCs there can be a window of time 
>> between weak oop marking where the oop is unmarked, so dead (the phantom 
>> load in peek returns NULL) but the gc_notification hasn't been done yet.  In 
>> this window, a heap walk or GetObjectsWithTags call would not find an object 
>> before the ObjectFree event is posted.  This is dealt with in two ways:
>> 
>> 1. In the Heap walk, there's an unconditional table walk to post events if 
>> events are needed to post.
>> 2. For GetObjectWithTags, if a dead oop is found in the table and posting is 
>> required, we use the VM thread to post the event.
>> 
>> Event posting cannot be done in a JavaThread because the posting needs to be 
>> done while holding the table lock, so that the JvmtiEnv state doesn't change 
>> before posting is done.  ObjectFree callbacks are limited in what they can 
>> do as per the JVMTI Specification.  The allowed callbacks to the VM already 
>> have code to allow NonJava threads.
>> 
>> To avoid rehashing, I also tried to use object->identity_hash() but this 
>> breaks because entries can be added to the table during heapwalk, where the 
>> objects use marking.  The starting markWord is saved and restored.  Adding a 
>> hashcode during this operation makes restoring the former markWord (locked, 
>> inflated, etc) too complicated.  Plus we don't want all these objects to 
>> have hashcodes because locking operations after tagging would have to always 
>> use inflated locks.
>> 
>> Much of this change is to remove serial weak oop processing for the 
>> weakProcessor, ZGC and Shenandoah.  The GCs have been stress tested with 
>> jvmti code.
>> 
>> It has also been tested with tier1-6.
>> 
>> Thank you to Stefan, Erik and Kim for their help with this change.
>
> Coleen Phillimore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Code review comments from Kim and Albert.

src/hotspot/share/prims/jvmtiTagMapTable.hpp line 36:

> 34: class JvmtiTagMapEntryClosure;
> 35: 
> 36: class JvmtiTagMapEntry : public HashtableEntry<WeakHandle, 
> mtServiceability> {

By using utilities/hashtable this buys into having to use HashtableEntry, which 
includes the _hash member, even though that value is trivially computed from 
the key (since we're using address-based hashing here). This costs an 
additional 8 bytes (_LP64) per entry (a 25% increase) compared to the old 
JvmtiTagHashmapEntry.  (I think it doesn't currently make a difference on 
!_LP64 because of poorly chosen layout in the old code, but fixing that would 
make the difference 33%).

It seems like it should not have been hard to replace the oop _object member in 
the old code with a WeakHandle while otherwise maintaining the Entry interface, 
allowing much of the rest of the code to remain the same or similar and not 
incurring this additional space cost.

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

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

Reply via email to