On Mon, 2 Nov 2020 15:58:15 GMT, Coleen Phillimore <[email protected]> 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 StefanK.
Some more nit-picking to make the code more consistent.
src/hotspot/share/prims/jvmtiTagMapTable.cpp line 52:
> 50: : Hashtable<WeakHandle, mtServiceability>(_table_size,
> sizeof(JvmtiTagMapEntry)) {}
> 51:
> 52:
Double whitespace
src/hotspot/share/prims/jvmtiTagMapTable.cpp line 185:
> 183: // Serially remove unused oops from the table, and notify jvmti.
> 184: void JvmtiTagMapTable::unlink_and_post(JvmtiEnv* env) {
> 185:
Stray newline
src/hotspot/share/prims/jvmtiTagMapTable.cpp line 224:
> 222: // Rehash oops in the table
> 223: void JvmtiTagMapTable::rehash() {
> 224:
Stray newline
src/hotspot/share/prims/jvmtiTagMapTable.hpp line 75:
> 73:
> 74: void resize_if_needed();
> 75: public:
Newline between
src/hotspot/share/prims/jvmtiTagMapTable.hpp line 100:
> 98: };
> 99:
> 100:
Double newline
src/hotspot/share/prims/jvmtiTagMapTable.cpp line 258:
> 256: int rehash_len = moved_entries.length();
> 257: // Now add back in the entries that were removed.
> 258: for (int i = 0; i < moved_entries.length(); i++) {
rehash_len is read, but not used in for loop condition.
src/hotspot/share/prims/jvmtiTagMapTable.cpp line 165:
> 163: }
> 164: }
> 165: const int _resize_load_trigger = 5; // load factor that will
> trigger the resize
Newline between
-------------
Marked as reviewed by stefank (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/967
