RFR: 8151670: Unexpected concurrent refinement deactivation and reactivation (original) (raw)
Jon Masamitsu jon.masamitsu at oracle.com
Wed Mar 23 16:25:25 UTC 2016
- Previous message (by thread): RFR: 8151670: Unexpected concurrent refinement deactivation and reactivation
- Next message (by thread): RFR: 8151670: Unexpected concurrent refinement deactivation and reactivation
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
On 03/23/2016 12:41 AM, Kim Barrett wrote:
On Mar 22, 2016, at 12:45 PM, Jon Masamitsu <jon.masamitsu at oracle.com> wrote:
Kim, Looks correct. I assume there will be a new version with a few changes from Mikael's comments. Thanks Jon. Replies below. New webrev: full: http://cr.openjdk.java.net/~kbarrett/8151670/webrev.01/ incr: http://cr.openjdk.java.net/~kbarrett/8151670/webrev.01.inc/
Kim,
Thanks for the change. All looks good.
Jon
Couple of minor points.
http://cr.openjdk.java.net/~kbarrett/8151670/webrev.00/src/share/vm/gc/g1/dirtyCardQueue.hpp.frames.html Extra "to" (end of 84 and beginning of 85). 84 // function. If "consume" is true, the node's index is updated to 85 // to exclude the processed elements, e.g. up to the element for Fixed. http://cr.openjdk.java.net/~kbarrett/8151670/webrev.00/src/share/vm/gc/g1/dirtyCardQueue.cpp.frames.html Instead of
186 assert(node->index() == buffersize(), "apply said fully consumed"); maybe something a little more wordy assert (node->index() == buffersize(), "Buffer was not fully processed as claimed: index " SIZEFORMAT " buffersize " SIZEFORMAT, node->index(), buffersize()); Would this assertion actually fit better at the end of 153 bool DirtyCardQueueSet::applyclosuretobuffer(CardTableEntryClosure* cl, 174 } assert (!result || node->index() == buffersize(), "Buffer was not fully processed as claimed: index " SIZEFORMAT " buffersize " SIZEFORMAT, node->index(), buffersize()); 175 return result; 176 } so that you don't need it in applyclosuretocompletedbuffer() and mutprocessbuffer()? I added an assertion helper that provides more information, and called from the two with the previous limited content asserts. I don’t think applyclosuretobuffer is the right place for it; it’s the other two places that are making decisions that will be wrong if that assertion doesn’t hold.
- Previous message (by thread): RFR: 8151670: Unexpected concurrent refinement deactivation and reactivation
- Next message (by thread): RFR: 8151670: Unexpected concurrent refinement deactivation and reactivation
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]