RFR: 8324649: Shenandoah: replace implementation of free set [v59]

[Y. Srinivas Ramakrishna]

On Mon, 13 May 2024 21:31:40 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:

>> Several objectives:
>> 1. Reduce humongous allocation failures by segregating regular regions from humongous regions
>> 2. Do not retire regions just because an allocation failed within the region if the memory remaining within the region is large enough to represent a LAB
>> 3. Track range of empty regions in addition to range of available regions in order to expedite humongous allocations
>> 4. Treat collector reserves as available for Mutator allocations after evacuation completes
>> 5. Improve encapsulation so as to enable an OldCollector reserve for future integration of generational Shenandoah
>> 
>> We have compared performance of existing FreeSet implementation with the proposed PR over a broad set of performance workloads and see that the impact is mostly neutral.
>> 
>> Comparing 105235.0 metrics from control, 220638.0 from experiment.
>> Compare: 0.589s
>>                           Most impacted benchmarks |                              Most impacted metrics
>> -------------------------------------------------------------------------------------------------------
>>                                  Shenandoah/jython |                                          cwr_total
>> 
>> 
>>                                 Only in experiment |                                    Only in control
>> -------------------------------------------------------------------------------------------------------
>>                  crypto.signverify/trigger_failure |                        crypto.rsa/cmr_thread_roots
>>                 extremem-large-31g/adjust_pointers |       scimark.sparse.small/concurrent_thread_roots
>>             extremem-large-31g/calculate_addresses |              xml.transform/concurrent_thread_roots
>>       crypto.signverify/class_unloading_rendezvous |                    mpegaudio/concurrent_weak_roots
>>                                   serial/cmr_total |                        crypto.rsa/ctr_thread_roots
>> 
>> Shenandoah
>> -------------------------------------------------------------------------------------------------------
>> +5.64% jython/cwr_total p=0.00037
>>   Control:      1.928ms (+/-272.40us)        170
>>   Test:         2.037ms (+/-322.73us)        344
>
> Kelvin Nilsen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix copyright notice on test file

src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 105:

> 103:   ShenandoahHeapRegion* r = _heap->get_region(idx);
> 104:   return can_allocate_from(r);
> 105: }

Some of this abstraction leakage can be avoided by making

"can_allocate_from(size_t idx)" a method on the `_heap` object, and calling that, letting the heap do the test:

``` 
bool ShenandoahHeap::can_allocate_from(size_t idx) {
  ShenandoahHeapRegion* r = get_region(idx);
  return r->is_empty() || (r->is_trash() && !is_concurrent_weak_root_in_progress());
}


with ShFreeSet merely delegating to its `_heap`.

Probably similar pattern for `alloc_capacity` method below.

(I'll keep leaving single comments, so I don't slow down the review/feedback across potential interruptions. I don't expect there will be many, but still.)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17561#discussion_r1599225270