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

[Kim Barrett]

On Sun, 28 Feb 2021 21:48:38 GMT, Albert Mingkun Yang <ayang 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
>
> src/hotspot/share/gc/shared/stringdedup/stringDedupTable.cpp line 405:
> 
>> 403:       size = round_up_power_of_2(needed);
>> 404:     } else {
>> 405:       size = _max_size;
> 
> It's not obvious to me which one is larger, btw `round_up_power_of_2(needed)` and `_max_size`.

This table implementation uses power of 2 sizes, including _min/max_size.
That's assumed throughout.  Since needed < _max_size, rounding needed up to
a power of 2 can't exceed _max_size.

> src/hotspot/share/gc/shared/stringdedup/stringDedupTable.cpp line 426:
> 
>> 424:     }
>> 425:   }
>> 426: 
> 
> How about adding an assertion, sth like, `asssert(size >= _min_size && size <= _max_size)`?

Sure, I can do that.

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

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