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

[Stefan Johansson]

On Mon, 1 Mar 2021 09:54:46 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
>
> src/hotspot/share/gc/shared/stringdedup/stringDedupTable.cpp line 411:
> 
>> 409:     size_t needed = _table->_entries / _shrink_load_factor;
>> 410:     if (needed > _min_size) {
>> 411:       size = round_down_power_of_2(needed);
> 
> Unfortunately, aggressive shrinking can't be done without other changes.
> The current resizing code assumes shrinking is only by a factor of 2,
> because of the way segments are claimed.  Shrinking could probably be
> adjusted to deal with larger factors, but I think that's outside the scope
> of what's needed for this change.  (Growth by an arbitrary power of 2
> doesn't seem to be a problem.)  Fixing...

I agree that it should be a separate change as long as the test failure is fixed. But I guess adjusting the grow should be enough to make the test pass, right?

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

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