RFR(M) 8222992: Shenandoah: Pre-evacuate all roots

Zhengyu Gu zgu at redhat.com
Fri Apr 26 13:54:30 UTC 2019


Hi Aleksey,

Thanks for reviewing.

On 4/26/19 9:02 AM, Aleksey Shipilev wrote:
> On 4/26/19 2:50 PM, Zhengyu Gu wrote:
>> Since we switched to strong to-space invariant, if we pre-evacuate all roots, the roots should only
>> contain to-space references, until next evacuation cycle. Therefore, we can avoid a couple of update
>> roots calls in regular paths. Also, make code easy to reason.
>>
>> The change may prolong final_mark pause. It is temporary. I intent to move several pre-evacuated
>> roots to concurrent phase, in followup RFEs.
>>
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8222992
>> Webrev: http://cr.openjdk.java.net/~zgu/JDK-8222992/webrev.00/
> 
> I'll take a more extensive look next week. Some brief observations here:
> 
> *) There is a plain store in ShenandoahBarrierSet::AccessBarrier<decorators,
> BarrierSetT>::oop_load_not_in_heap. First, I wonder if it even makes sense to do it here -- it feels
> awkward to have writes in "loads". Second, shouldn't that be a CAS, on the off-chance something else
> is storing the another value?

I think it is a Copy-on-Read semantics, no? I don't think it needs CAS 
here, since load-reference-barrier should return to-space oop, and we 
should only have one to-space copy.

> 
> *) I wonder if verify_roots() and the associated closure belongs to Verifier (not necessarily
> protected by ShenandoahVerify) or shenandoahAsserts. In fact, maybe we should just strategically
> rely on Verifier to catch non-forwarded roots?

Can do this.

> 
> *) In final-marking, the assert(task_queues()->is_empty(), "Should be empty"); is gone, why?
My mistake, will revert this part.


> 
> *) Can the per-line DEBUG_ONLY in shenandoahPhaseTimings be turned into #ifdef DEBUG block?
Could not get this to work, cause they are embedded inside a macro :-)

> 

> *) Typo here, "Verfiy":
> 
>    322   DEBUG_ONLY(f(verify_roots,                        "Verfiy Roots"))                    \

Will fix this.
> 
> -Aleksey
> 
> 
> 



More information about the hotspot-gc-dev mailing list