On Mon, 2 Nov 2020 15:58:15 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 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