request for review (M), 6458402: 3 jvmti tests fail with CMS and +ExplicitGCInvokesConcurrent

[Y. Srinivas Ramakrishna]

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