RFR: 8272609: Add string deduplication support to SerialGC [v10]

[Kim Barrett]

On Fri, 20 Aug 2021 21:17:50 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> Denghui Dong has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fix build error
>
> src/hotspot/share/gc/serial/defNewGeneration.cpp line 606:
> 
>> 604:   assert(heap->no_allocs_since_save_marks(), "save marks have not been newly set.");
>> 605: 
>> 606:   _string_dedup_requests.flush();
> 
> I wonder if placing `_string_dedup_requests.flush();` before weak roots processing (`WeakProcessor::weak_oops_do`) makes more senses. The documentation says `flush` should be called when marking is done; for Serial young GC, it's equivalent to when evacuation is done, which is right after ref-processing.

That documentation is mistaken; that's a left-over from an earlier development version that I forgot to update.  Without special additional care, the lifetime also needs to cover final-reference processing.  I have a todo item to file a bug against that comment.

> src/hotspot/share/gc/serial/markSweep.cpp line 219:
> 
>> 217:   MarkSweep::_gc_timer = new (ResourceObj::C_HEAP, mtGC) STWGCTimer();
>> 218:   MarkSweep::_gc_tracer = new (ResourceObj::C_HEAP, mtGC) SerialOldTracer();
>> 219:   MarkSweep::_string_dedup_requests = new StringDedup::Requests();
> 
> It's not clear to me why `_string_dedup_requests` is a pointer. Is it possible to define `static StringDedup::Requests _string_dedup_requests;` directly? That way, `Requests` doesn't need to use the inheritance.

It's a non-local variable with a non-trivial destructor.  See
https://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2021-August/036412.html

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

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