RFR: Process remaining SATB buffers in final mark/traverse loop instead of separate phase

[Aleksey Shipilev]

On 06/19/2018 07:45 PM, Aleksey Shipilev wrote:
> On 06/19/2018 07:22 PM, Roman Kennke wrote:
>> We don't need an extra phase to process remaining complete buffer in
>> final-mark/-traverse, and we don't need all the loop versions twice with
>> and without SATB. We can just as well do that in one single loop, pretty
>> much the same as conc-mark/-traversal. The important part is to process
>> *in*complete thread-local buffers. We already piggy-back this on
>> final-thread-scanning in traversal, and we keep a small extra phase for
>> that in conc-mark. At the very least, this should keep the overall
>> generated code smaller.
>>
>> Aleksey, can you give this some smoke performance runs to check for
>> possible regressions or maybe improvements?
>>
>> http://cr.openjdk.java.net/~rkennke/satb-final-phase/webrev.00/
> 
> Looks good, except... The thing about code like below, is that it is deliberately racy: we are
> checking completed_buffers_num without taking the SATB mutex:
> 
> 1002     while (satb_mq_set.completed_buffers_num() > 0) {
> 1003       satb_mq_set.apply_closure_to_completed_buffer(&drain_satb);
> 1004     }
> 
> ...which means, we can miss the complete buffer while there is actually one. It does not matter for
> the concurrent cycle, because we would either see it on next marking stride, or final-* would drain
> everything without checking for this flag.
> 
> But this changes once we remove this line from SCM::drain_satb_buffers():
> 
> 507  while (satb_mq_set.apply_closure_to_completed_buffer(&cl));
> 
> Now, we at the mercy of that racy check to drain the completed buffers. I would not vouch the whole
> thing works correctly then. It seems that leaving that line alone provides us with guarantees for
> final-mark. Not sure if the same applies to Traversal.
> 
> So, I think we can drop DRAIN_SATB template parameter and simplify marking loops then, but not
> really remove the unconditional SATB draining that guarantees finishing SATBs.

Also, I think you can inline drain_satb_buffers(), because there seems to be the only use.

-Aleksey