RFR: Make weakref processing always use Executor

Roman Kennke rkennke at redhat.com
Fri Aug 10 16:47:34 UTC 2018


Am 10.08.2018 um 16:05 schrieb Aleksey Shipilev:
> On 08/10/2018 03:40 PM, Roman Kennke wrote:
>>>> I am passing NULL for all the single-threaded-closures because they are
>>>> no longer needed. Because of this, all the
>>>> process_discovered_references() call are the same and can be moved out
>>>> of the if-else-tree.
>>>
>>> Well, *that* should be expressed in the comment near the appropriate block, because you remember it
>>> today, but would completely forget in half a year. Also need assert to nail down the precondition
>>> for this to work. Something like this:
>>>
>>>    // Process references runs in executor workers only. This means we do not
>>>    // need to pass any closures to it, because task proxy would set up its own.
>>>    assert(rp->processing_is_mt(), "sanity");
>>>    rp->process_discovered_references(NULL, NULL, NULL,
>>>                                      &executor, &pt);
>>>    pt.print_all_references();
>>
>> Well, the thing is that RP decides on phase-by-phase-basis to change
>> mt-ness. The assert would not catch that. Maybe pass
>> 'should-not-reach-here-closures'  instead of NULL?
> 
> Wait a minute. The code relies on RP always entering the MT path when Shenandoah is enabled, even
> though task proxy would fake it before actually setting up the threads, right? Which means
> processing_is_mt() should be true on all Shenandoah paths? My intent with that assert was to assert
> we always enter the task proxy path. ShouldNotReachable-closures would be more straightforward way
> to assert this.

I've added ShouldNotReachHere*Closures in the relevant places and fixed
the other stuff you've mentioned.
I've also run tier3_gc_shenandoah and good that I did: there's a special
case when -XX:-ParallelRefProcEnabled where we used to turn off
mt_processing wholesale. I've fixed that to always do mt_processing but
with only 1 thread (which is 100% equivalent with this patch).

This passes tier3_gc_shenandoah now. specjvm is currently running, and
looks good so far.

Incremental:
http://cr.openjdk.java.net/~rkennke/weakrefs-1thread/webrev.01.diff/
Full:
http://cr.openjdk.java.net/~rkennke/weakrefs-1thread/webrev.01/

Good now?
Cheers,
Roman


More information about the shenandoah-dev mailing list