RFR: 8261859: gc/g1/TestStringDeduplicationTableRehash.java failed with "RuntimeException: 'Rehash Count: 0' found in stdout" [v2]

[Stefan Johansson]

On Mon, 1 Mar 2021 10:30:12 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Please review this fix of an intermittently failing string deduplication test.
>> 
>> There are several problems.
>> 
>> (1) Because of environmental or other unrelated changes, the test might
>> simply fail.  The test was considering it a failure if any GC reported a
>> zero rehash count.  But if the first GC triggered a resize, that would
>> suppress the requested "rehash a lot", and could report a zero rehash count,
>> failing the test.  So the test criteria is changed to require at least one
>> non-zero rehash count rather than no zero rehash counts.  Since rehashes are
>> normally unlikely, and the primary point is to exercise the rehash code,
>> having some reported non-zero rehash count is sufficient.
>> 
>> (2) Reporting only occurs if the string dedup thread was triggered and had
>> work to do.  If the initial collections all need resizes, and none of the
>> subsequent ones have any work for the thread to do, then again we won't have
>> any reported rehashes.  The test is changed to always generate some new
>> strings for later GCs to discover and queue for deduplication processing,
>> causing the dedup thread to run and reporting at the end.
>> 
>> (3) The table resizing mechanism was only doing one step (doubling or
>> halving) of size change per collection.  If the number of table entries is
>> large (small), several GCs might be required for the table to grow (shrink)
>> to the desired size.  Once again, this could suppress table rehashes,
>> causing the test to fail.  It also wastes effort because the table needs to
>> be resized multiple times when one right-sized resize would be sufficient.
>> Resizing now computes the "final" size based on the number of entries and
>> load factors, and may increase or decrease the table size by multiple powers
>> of 2 in one resizing operation.
>> 
>> Testing:
>> Manual execution of the string dedup tests and examining their logs.
>> 
>> Manual execution of the resize and rehash string dedup tests with a small
>> initial table size, to similate an environment with a larger initial set of
>> strings that triggers early resize.
>> 
>> mach5 tier1
>
> Kim Barrett has updated the pull request incrementally with one additional commit since the last revision:
> 
>   limit shrink, add asserts for new size

src/hotspot/share/gc/shared/stringdedup/stringDedupTable.cpp line 410:

> 408:     // Compute new size.  We can't shrink by more than a factor of 2,
> 409:     // because the partitioning for parallelization doesn't support more.
> 410:     if (size > _min_size) size /= 2;

I would prefer braces :) But an even nicer solution would be to use `clamp(...)` both for the grow and shrink case. What do you think about that?

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

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