Hi Keith -- This is perfect! Thanks for all the consolidation and clean-up!
PS: i believe this should fix the recently noted nightly failure in G1 (probably new since G1 started doing weak reference processing recently! -- i had seen it earlier with CMS, but have not checked for the existence of an open CR). Here's the failure:- http://sqeweb.sfbay.sun.com/nfs/tools/gtee/results/JDK7/NIGHTLY/VM/2010-12-29/G1_GC_Baseline-tiered/vm/linux-amd64/server/mixed/linux-amd64_vm_server_mixed_nsk.hprof.testlist/ResultDir/HeapThrasher/hs_err_pid21399.log Could you test yr workspace with this test and note that it's been fixed by yr changes? OK, I just checked and it's this one:- 6729714 intermittent failures of assert(orig_key != 0L,"jni weak reference cleared!!") I am guessing you can close that as a dup of your bug fix here as these changes will take care of that bug as well. thanks! -- ramki On 12/30/2010 3:03 PM, Keith McGuigan wrote:
Hello again, My last request for a code review this year, I promise! This fix adds the missing JVMTI GC callbacks for CMS: Webrev: http://cr.openjdk.java.net/~kamg/6458402/webrev.03/ Details: Existing mechanism used a stack-allocated JvmtiGCMarker to bracket the GC operation. The destructor, in addition to posting the gc-end event, also did come bookkeeping work for JvmtiBreakpoints and JvmtiTagMap. Unfortunately, this cleanup work needs to be done at safepoint, but for CMS, gc-end may not happen at safepoint, so the JVMTI gc-end event posting has to be decoupled from the other work to update the data structures. I changed the mechanism so that the bookkeeping work could be done during the oop-iteration phase instead of being invoked from the JvmtiGCMarker destructor. This allowed the JvmtiGCMarker to be used outside of a safepoint. The bookkeeping code was modified to operate during oop iteration. The changes to update the JvmtiBreakpoint cache was rather trivial, but the changes to JvmtiTagMap were more involved. The JVMTI tag map code had maintained a separate hashtable of JNI weak references for each generation, and rehashed the required tables after a GC. I unified these into a single table containing raw oops instead, and added a weak oop iteration over the single table instead. Only oops that moved need to be rehashed, and this is now done in-line rather than as a full rehash step at the end of GC. The separate tables were intended to reduce the work done during a minor GC, but since each entry contained a weak reference, all tagged objects needed to be visited in JNIHandles::weak_oops_do() anyway. Using one table and iterating the contained oops as weak roots appears to work well too, as only the entries with oops that move are rehashed. So during a minor collection, all oops will be seen (as before in JNIHandles), but only the oops in the young generation will move and need to be rehashed. A sample program that tagged 8M objects (half in each generation) showed a 10+% performance improvement in time spent in safepoint. During addition of the JVMTI callbacks, I noticed the DTrace callbacks were missing for CMS too. I ran with Tony's idea of putting the DTrace probes in a marker object and unified it so there's only one marker to use which performs the work needed for JVMTI as well as for DTrace. I added this as SvcGCMarker and replaced the locations of JvmtiGCMarker and notify_gc_[begin/end] with the new marker. -- - Keith
