Request for Review (s) - 8149343: assert(rp->num_q() == no_of_gc_workers) failed: sanity (original) (raw)

Kim Barrett kim.barrett at oracle.com
Wed Mar 16 05:59:56 UTC 2016

On Mar 10, 2016, at 2:53 PM, Jon Masamitsu <jon.masamitsu at oracle.com> wrote:

https://bugs.openjdk.java.net/browse/JDK-8149343 The error here was that the number of active workers was not always set correctly for parallel Reference processing. The first fix was to set the number of active workers in the ReferenceProcessor in the task constructor. Once thatDiff was fixed a subsequent assertion failure occurred. # Internal Error (/export/jmasa/java/jdk9-rt-8149343/src/share/vm/gc/shared/referenceProcessor.cpp:884), pid=18444, tid=18500 # assert(id < maxnumq) failed: Id is out-of-bounds id 23 and max id 23)_ _This was fixed by the change_ _- if (++nextid == numq) {_ _+ assert(!discoveryismt, "Round robin should only be used in serial discovery");_ _+ if (++nextid >= numq) { See the CR for an example log which showed numq changing values between different phases of a collection and where the value of nextid was greater than numq. The last change was to add a parameter to the logging function so that the logging only output the active lists (which made the reading of the logs simpler) - void logreflistcounts(DiscoveredList reflists[], sizet totalcount) PRODUCTRETURN; + void logreflistcounts(DiscoveredList reflists[], uint activelength, sizet totalcount) PRODUCTRETURN; This patch fixes UseG1GC for the failure where UseDynamicNumberOfGCThreads and ParallelRefProcEnabled are both turned on. There is still a failure with UseParallelGC that is being fixed under 8150994. http://cr.openjdk.java.net/~jmasa/8149343/webrev.00/ Testing: gctestsuite with UseDynamicNumberOfGCThreads on and ParallelRefProcEnabled on and off. rbt in progress

Looks good.

I agree with Thomas about the oddness of using >= in the _next_id wraparound check. It seems like the place to initialize it is in set_active_mt_degree. Oh, I see you are already considering that. I would also be ok with just a comment in next_id().

src/share/vm/gc/shared/referenceProcessor.cpp 705 for (uint i = 0; i < active_length; ++i) {

For paranoia, perhaps afterward loop from active_length below _max_num_q and report any unexpected non-empty lists.

test/gc/ergonomics/TestDynamicNumberOfGCThreads.java

Copyright.

src/share/vm/gc/shared/referenceProcessor.cpp 723 total_refs += ref_lists[i].length(); 724 } 725 log_reflist_counts(ref_lists, _max_num_q, total_refs);

788 balanced_total_refs += ref_lists[i].length(); 789 } 790 log_reflist_counts(ref_lists, _num_q, balanced_total_refs);

More information about the hotspot-gc-dev mailing list