RFR: 8267188: gc/stringdedup/TestStringDeduplicationInterned.java fails with Shenandoah

[Aleksey Shipilev]

On Tue, 17 Aug 2021 18:32:54 GMT, Zhengyu Gu <zgu at openjdk.org> wrote:

> Shenandoah currently enqueues deduplication candidates when marks object gray.
> 
> However, it can not do so when it scans roots, as it can potentially result lock rank inversion between stack watermark lock and dedup request buffer lock.
> 
> As the result, it can not enqueue as many as candidates as it should be able to, I believe JDK-8271834 is due to the same problem. I purpose we switch to enqueue candidates when we mark object black.
> 
> We are still not able to enqueue all candidates, only when they have displaced headers or have monitor inflating in progress. Upstream is working on removing displaced headers, we should revisit the logic afterward, or we can choose to deduplicate all string regardless their ages.
> 
> 
> 
> Test:
> - [x] hotspot_gc_shenandoah
> - [x] tier1 with -XX:+UseShenandoahGC and -XX:+UseStringDeduplication

Looks fine, modulo a few suggestions.

src/hotspot/share/gc/shenandoah/shenandoahMark.cpp line 120:

> 118: }
> 119: 
> 120: template <class T, bool CANCELLABLE,StringDedupMode STRING_DEDUP>

Suggestion:

template <class T, bool CANCELLABLE, StringDedupMode STRING_DEDUP>

src/hotspot/share/gc/shenandoah/shenandoahMark.inline.hpp line 71:

> 69:   if (task->is_not_chunked()) {
> 70:     if (obj->is_instance()) {
> 71:       dedup_string<STRING_DEDUP>(obj, req);

I do wonder if it is faster to _first_ let the closure process the outbound references, and then do dedup for the object? This way, parallel GC workers can do work while we are calling `dedup_string` here. This is one of the reasons why old code did dedup check after the queue push. In other words, move this call at the end of this block.

-------------

Marked as reviewed by shade (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5151