RFR: 8338535: GenShen: some style improvements to source code implementation [v3]

Aleksey Shipilev shade at openjdk.org
Tue Aug 20 09:03:12 UTC 2024


On Tue, 20 Aug 2024 00:05:33 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:

>> 1. Hide union data structure behind accessor methods for ShenandoahHeuristics::RegionData._u
>> 2. Change type of ShenandoahCollectionSet::establish_preselected from byte* to byte[] to clarify intended usage.
>> 3. Remove unhelpful comment from shenandoahFullGC.cpp
>
> Kelvin Nilsen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Code touchup
>   
>   Remove 2 unneeded blank lines
>   Move and simplify a comment from call point to called method
>   implementation

Understand the intent: it is good to have guardrail that checks the tags. Stylistic nits:

src/hotspot/share/gc/shenandoah/heuristics/shenandoahAdaptiveHeuristics.hpp line 73:

> 71: 
> 72:   virtual void choose_collection_set_from_regiondata(ShenandoahCollectionSet* cset,
> 73:                                                      RegionData data[], size_t size,

Not at all sure this is a great style. It is also not consistent with the rest of Hotspot: searching for `[],` or `[] ` in `.cpp` files yields no hits. Leave it be at `RegionData*`.

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

PR Review: https://git.openjdk.org/shenandoah/pull/475#pullrequestreview-2247391390
PR Review Comment: https://git.openjdk.org/shenandoah/pull/475#discussion_r1722952459


More information about the shenandoah-dev mailing list