RFR: 8324649: Shenandoah: refactor implementation of free set [v38]

Roman Kennke rkennke at openjdk.org
Tue Mar 26 16:48:35 UTC 2024


On Tue, 26 Mar 2024 16:01:04 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 with a new target base due to a merge or a rebase. The pull request now contains 94 commits:
> 
>  - Merge remote-tracking branch 'origin/master' into restructure-free-set
>  - Merge branch 'openjdk:master' into master
>  - Merge branch 'openjdk:master' into master
>  - Fix for certain compilers
>  - Add a few more conditions to ShenandoahSimpleBitMapTest
>  - Fix typo in log message for 32-bit platforms
>  - Fix one bug and add disabled instrumentation for debugging
>  - Give special handling to count-zeros when arg is 0
>  - Exploit count_<leading|trailing>_zeros instructions
>    
>    The ShenandoahSimpleBitMap can run more efficiently if we use hardware
>    instructions to count leading and trailing zeros.
>  - Remove comment that pertains only to generational mode
>  - ... and 84 more: https://git.openjdk.org/jdk/compare/da8a095a...7bc418fa

Please remove the KELVIN_DEBUG stuff, otherwise this is hard to review. ;-)

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

> 51:   uintx omit_mask = right_n_bits(bit_number);
> 52:   uintx mask = ((uintx) 0 - 1) & ~omit_mask;
> 53: #undef KELVIN_DEBUG

Please remove KELVIN_DEBUG before integration. You may want to turn it into proper logging, if you think it may be useful.

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

Changes requested by rkennke (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17561#pullrequestreview-1961091861
PR Review Comment: https://git.openjdk.org/jdk/pull/17561#discussion_r1539704512


More information about the shenandoah-dev mailing list