RFR: Enable string deduplication in all marking phases

[Zhengyu Gu]

On 10/02/2018 12:07 PM, Aleksey Shipilev wrote:
> On 10/02/2018 05:51 PM, Zhengyu Gu wrote:
>> Aleksey found that Shenandoah is inconsistent on honoring string deduplication flag.
>>
>> Shenandoah string deduplication was initially designed for concurrent marking phases only. Over the
>> courses, it diverged. e.g. Traversal GC also performs deduplication in init and final pauses.
>>
>> So, let's make it consistent:
>>
>> Webrev: http://cr.openjdk.java.net/~zgu/shenandoah/dedup_all_phases/webrev.00/
> 
> Good stuff.
> 
> *) code_blobs became unused, and instead we are always going via MarkingCodeBlobClosure on product
> path? That's very unfortunate. Can we maybe pass the ShenandoahInitMarkRootsClosure into the
> do_work, and leave the rest of the code intact? Or even specialize ShenandoahInitMarkRootsTask with
> STRING_DEDUP?

Right, it does not handle concurrent code root scan properly.

It looks like concurrent code root scan not working well with 
StringDeduplication, due to lock ranking inversion. I propose that we 
simply ignore possible candidates in code root for now.

Webrev: 
http://cr.openjdk.java.net/~zgu/shenandoah/dedup_all_phases/webrev.01/

Test:

   Reran tier3_gc_shenandoah.

Thanks,

-Zhengyu

> 
> *) If thing above is not an issue, let's clean up this block a bit:
>      - L105 needs the space after comma.
>      - "cldCl" and "blobsCl" need to be "cld_cl" and "blobs_cl", respectively
> 
>   104     if (ShenandoahStringDedup::is_enabled()) {
>   105       ShenandoahInitMarkRootsClosure<UPDATE_REFS,ENQUEUE_DEDUP> mark_cl(q);
>   106       CLDToOopClosure cldCl(&mark_cl);
>   107       MarkingCodeBlobClosure blobsCl(&mark_cl, ! CodeBlobToOopClosure::FixRelocations);
>   108       do_work(heap, &mark_cl, _process_refs ? NULL : &mark_cl, &cldCl, &blobsCl, worker_id);
>   109     } else {
>   110       ShenandoahInitMarkRootsClosure<UPDATE_REFS, NO_DEDUP> mark_cl(q);
>   111       CLDToOopClosure cldCl(&mark_cl);
>   112       MarkingCodeBlobClosure blobsCl(&mark_cl, ! CodeBlobToOopClosure::FixRelocations);
>   113       do_work(heap, &mark_cl, _process_refs ? NULL : &mark_cl, &cldCl, &blobsCl, worker_id);
>   114     }
> 
> 
> Thanks,
> -Aleksey
>