RFR: 8151670: Unexpected concurrent refinement deactivation and reactivation

[Jon Masamitsu]

Kim,

Looks correct.  I assume there will be a new version with a few
changes from Mikael's comments.

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

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()?

Jon

On 3/18/2016 4:10 PM, Kim Barrett wrote:
> Please review this change to the concurrent refinement threads to use
> SuspendibleThreadSet::yield, rather than deactivation, in response to
> an STS suspension request. This should mostly reduce the cost of
> coming out of a non-GC safepoint. See the CR for further discussion.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8151670
>
> Webrev:
> http://cr.openjdk.java.net/~kbarrett/8151670/webrev.00/
>
> Testing:
> JPRT, RBT GC Nightly, local specjbb2015.
>
>