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

[Kelvin Nilsen]

On Fri, 1 Mar 2024 17:57:49 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Kelvin Nilsen has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 60 commits:
>> 
>>  - Remove instrumentation and cleanup magic numbers
>>    
>>    Two places, I had used 63 when I should have used
>>    _bits_per_array_element -1.
>>  - Address 32-bit compile issues
>>  - Fix 32-bit size formatting in log messages
>>  - Fix white space
>>  - Merge remote-tracking branch 'origin/master' into restructure-free-set
>>  - Merge branch 'openjdk:master' into master
>>  - Two bug fixes for better performance
>>    
>>    1. Collector reserve size is based on memory available within regions
>>       rather than the region size (oops).
>>    
>>    2. If an attempt to allocate within a region fails and the region has
>>       already provided the percentage presumed by ShenandoahEvacWaste,
>>       retire this region.  This is motivated by observations that
>>       otherwise, we end up with large numbers of regions that have only
>>       a small amount of memory within them (e.g. 4k) and every allocation
>>       request has to wade through all of these regions before it eventually
>>       finds a region that has a sufficiently large amount of available
>>       memory.  In the original Shenandoah free-set implementation, the
>>       behavior was to retire the first time an allocation within that
>>       regions fails, regardless of whether the region has already reached
>>       the ShenandoahEvacWaste threshold.
>>  - Fix off-by-one error in is_forward_consecutive_ones()
>>  - Fix whitespace
>>  - Bug fixes and performance improvements
>>    
>>    1. Correct off-b-one-error in count of trailingones
>>    2. Speed up search for contiguous regions (for humongous allocations) by
>>       sliding window instead of initiating new search each time
>>    3. Bias regular region allocations to favor regions that are already
>>       partially consumed
>>    4. Fix bug in move_regions_from_collector_to_mutator which caused some
>>       non-empty regions to be ignored.
>>  - ... and 50 more: https://git.openjdk.org/jdk/compare/be2b92bd...1aa5a3e6
>
> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 1372:
> 
>> 1370:     if (move_to_collector) {
>> 1371:       // Note: In a previous implementation, regions were only placed into the survivor space (collector_is_free) if
>> 1372:       // they were entirely empty.  I'm not sure I understand the rationale for that.  That alternative behavior would
> 
> We should not leave "I'm not sure..." comments in the code. I believe the reason is exactly what you stated below: we do not want to put evacuated objects (implicitly live) to the region that might be mostly garbage, thus prolonging the garbage lifetime.

Thanks.  I've rewritten the comment in the affirmative.

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

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