RFR: Shenandoah String Dedup refactoring

Zhengyu Gu zgu at redhat.com
Tue Jul 3 17:38:46 UTC 2018


Thanks for the review.

On 07/03/2018 11:53 AM, Aleksey Shipilev wrote:
> On 07/02/2018 07:51 PM, Zhengyu Gu wrote:
>> Webrev: http://cr.openjdk.java.net/~zgu/shenandoah/shared_stringdedup/webrev.02/
> 
> Nice to see another upstream diff ripout!
> 
> *) Should we split out ShenandoahWorkerSession into a separate changeset for backporting?

Split into two changesets:

Worker session: 
http://cr.openjdk.java.net/~zgu/shenandoah/worker_id/webrev.00/

String dedup: 
http://cr.openjdk.java.net/~zgu/shenandoah/shared_stringdedup/webrev.03/



> 
> *) In shenandoahHeap.cpp, this does not seem to be needed:
>   2196   ShenandoahHeap* heap = ShenandoahHeap::heap();
Fixed.



> 
> *) Use locals:
> 
>   105   static inline uint worker_id() {
>   106     Thread* thr = Thread::current();
>   107     assert(ShenandoahThreadLocalData::worker_id(thr) !=
> ShenandoahThreadLocalData::INVALID_WORKER_ID, "Worker session has not been created");
>   108     return ShenandoahThreadLocalData::worker_id(thr);
>   109   }
> 
> ...
> 
> static inline uint worker_id() {
>    Thread* thr = Thread::current();
>    id = ShenandoahThreadLocalData::worker_id(thr);
>    assert(id != ShenandoahThreadLocalData::INVALID_WORKER_ID, "Worker session has not been created");
>    return id;
> }
Fixed.

Test:

   Reran tier3_gc_shenandoah.

Thanks,

-Zhengyu


> 
> Thanks,
> -Aleksey
> 


More information about the shenandoah-dev mailing list