RFR: 8342001: GenShen: Factor cases for allocation type into separate methods [v2]

[William Kemper]

On Mon, 14 Oct 2024 20:16:17 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> William Kemper has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Improve comments for new methods
>
> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 762:
> 
>> 760: 
>> 761: HeapWord* ShenandoahFreeSet::allocate_for_mutator(ShenandoahAllocRequest &req, bool &in_new_region) {
>> 762:   maybe_change_allocation_bias();
> 
> Why update alloc bias if allocation is doomed to fail at next line? Is that the intention? If not, may be do the bias update only when there is chance for success in the allocation? See suggestions further below.

I had this same thought, but ended on the side of preserving existing behavior. @kdnilsen , should we not update the allocation bias if we can detect an early allocation failure?

> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 799:
> 
>> 797:                                 - _partitions.rightmost_empty(ShenandoahFreeSetPartitionId::Mutator));
>> 798:     _partitions.set_bias_from_left_to_right(ShenandoahFreeSetPartitionId::Mutator, (non_empty_on_right < non_empty_on_left));
>> 799:     _alloc_bias_weight = _InitialAllocBiasWeight;
> 
> The value of 256 for `_InitialAllocBiasWeight` seems a bit of magic. How was it chosen, and is it subject to bit-rot with code evolving? I can imagine that the bias update at a higher frequency is apt to not matter much, and since the leftmost and rightmost empty calculation is not free, so larger values are favored.
> 
> Also should we use the more usual `static const ssize_t INITIAL_ALLOC_BIAS_WEIGHT` Hotspot style for constants, rather than the camel case with an initial underscore (which seems not common)?

I'm not sure where the initial value comes from. I will change to the idiomatic name convention.

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

PR Review Comment: https://git.openjdk.org/shenandoah/pull/511#discussion_r1800183845
PR Review Comment: https://git.openjdk.org/shenandoah/pull/511#discussion_r1800184751