RFR: 8342001: GenShen: Factor cases for allocation type into separate methods [v2]
Y. Srinivas Ramakrishna
ysr at openjdk.org
Mon Oct 14 21:25:33 UTC 2024
On Mon, 14 Oct 2024 20:12:24 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 774:
>
>> 772: }
>> 773:
>> 774: return allocate_from_right_to_left(req, in_new_region);
>
> Just a thought: Might make the code easier to read, maintain, and reason about, if instead of a boolean value indicating left bias, if we used a two value enum: LeftBias, RightBias, and used an `_alloc_bias` array for the partitions.
>
> Then code might read:
>
> .... set_alloc_bias(LeftBias); // set allocation bias to left
>
> and the above code might look like:
>
> if (_partitions.is_empy(SFSPI::Mutator)) {
> return nullptr;
> }
> update_alloc_bias();
> switch (_partitions._alloc_bias[ShenandoahPartitionId::Mutator]) {
> case LeftBias: return allocate_from_left(req, in_new_region);
> case RightBias: return allocate_from_right(req, in_new_region);
> default: ShouldNotReachHere();
> }
I also wonder if `allocate_from_left` etc., since they seem to involve only mutator allocations, should include `mutator` somewhere in the name of the method; if so, I wonder about why we maintain the bias variable for the other partitions? (I suppose we don't but since each is a partition and the variable is associated with each partition, it's there. If we don't think we'll need bias for other allocations, it might make sense to move it to the free set, but I'd hold off on that until it's clear that directional bias for allocation isn't going to be useful for collector allocations.
-------------
PR Review Comment: https://git.openjdk.org/shenandoah/pull/511#discussion_r1800129152
More information about the shenandoah-dev
mailing list