RFR: Fixed two string dedup issues

[Aleksey Shipilev]

On 10/10/2017 06:52 PM, Zhengyu Gu wrote:
> 
> 
> On 10/10/2017 12:36 PM, Aleksey Shipilev wrote:
>> On 10/10/2017 06:35 PM, Zhengyu Gu wrote:
>>> On 10/10/2017 12:00 PM, Aleksey Shipilev wrote:
>>>> On 10/10/2017 03:47 PM, Zhengyu Gu wrote:
>>>>> Updated webrev: http://cr.openjdk.java.net/~zgu/shenandoah/strdedup_fix/webrev.01/index.html
>>>> Can you explain a little more why do we need to filter for only Shenandoah safepoints? The string
>>>> would only be queued, why does it matter if we are not in Shenandoah safepoint?
>>> Okay, added comments.
>>>
>>
>> I mean, can you explain here so I can understand what failure scenario you have in mind?
> 
> String dedup protocol requires queuing during GC safepoints (where it suspends deplication thread)
> and deduplicating strings outside GC safepoints.

That is what I don't understand. G1StringDedupQueue says:

// Pushing to the queue is thread safe (this relies on each thread using a unique worker
// id), but only allowed during a safepoint. Popping from the queue is NOT thread safe
// and can only be done by the deduplication thread outside a safepoint.

Notice "safepoint", not "GC safepoint". Where "GC safepoint" is coming from?

> G1 does all queuing during evacuation at GC safepoints, just as what we do with full GC.

Well, of course, it is STW. But it is non-sequitur why we need to check for *GC* safepoint for
Shenandoah.

> Shenandoah evacuates objects inside and outside GC safepoints, so we need to know if it is a GC or
> none GC safepoint, we only queue strings if it is a GC safepoint, otherwise, we disregard the
> safepoint.

Look, you are explaining *what* happens in the code patch. I am asking *why* it happens. Notably,
*why* the distinction between GC safepoints and non-GC safepoints matter? Why does it matter to
check for Shenandoah-specific safepoint? Why can't we accept the push to dedup queue from the
generic safepoint?

-Aleksey