RFR: Make weakref processing always use Executor

Aleksey Shipilev shade at redhat.com
Fri Aug 10 14:05:23 UTC 2018


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.

-Aleksey



More information about the shenandoah-dev mailing list