RFR: 8151670: Unexpected concurrent refinement deactivation and reactivation

Jon Masamitsu jon.masamitsu at oracle.com
Wed Mar 23 16:25:25 UTC 2016



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() == buffer_size(), "apply said fully consumed");
>> maybe something a little more wordy
>>
>> assert (node->index() == buffer_size(), "Buffer was not fully processed as claimed: index " SIZE_FORMAT " buffer_size " SIZE_FORMAT, node->index(), buffer_size());
>>
>> Would this assertion actually fit better at the end of
>>
>> 153 bool DirtyCardQueueSet::apply_closure_to_buffer(CardTableEntryClosure* cl,
>>
>> 174   }
>>
>> assert (!result || node->index() == buffer_size(), "Buffer was not fully processed as claimed: index " SIZE_FORMAT " buffer_size " SIZE_FORMAT, node->index(), buffer_size());
>> 175 return result;
>> 176 }
>>
>> so that you don't need it in apply_closure_to_completed_buffer()
>> and mut_process_buffer()?
> I added an assertion helper that provides more information, and called from the two
> with the previous limited content asserts.  I don’t think apply_closure_to_buffer 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