RFR: 8151670: Unexpected concurrent refinement deactivation and reactivation (original) (raw)

Mikael Gerdin mikael.gerdin at oracle.com
Wed Mar 23 13:00:51 UTC 2016

Hi Kim,

On 2016-03-23 08:41, 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/

I'm ok with this version.

One thing, though. 178 inline void assert_fully_consumed(BufferNode* node, size_t buffer_size) { Moving assertions to inline functions is problematic because on Windows we cannot print the stack trace from an assertion context, so if this assert fires off on Windows there is no way to know which caller triggered it since the line number will always point to the inline function.

Would you be ok with making this a macro or passing in LINE to assert_fully_consumed and print something like "Caller at " FILE ":%d", line

I don't need to see a new webrev for either of those approaches if you decide to follow them.

/Mikael

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.

More information about the hotspot-gc-dev mailing list