RFR: 8342560: GenShen: Fix confusing method name
Y. Srinivas Ramakrishna
ysr at openjdk.org
Thu Oct 17 21:49:24 UTC 2024
On Thu, 17 Oct 2024 20:56:59 GMT, William Kemper <wkemper at openjdk.org> wrote:
> Rename `is_old` to make it more clear that this about pointers that are outside the scope of a young collection.
A couple of comments. Will re-review again after your response. Thanks!
src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp line 606:
> 604: inline bool is_in_young(const void* p) const;
> 605: inline bool is_in_old(const void* p) const;
> 606: inline bool is_not_in_active_young_collection(oop obj) const;
Needs a single line documentation comment, given its not very natural/intuitive semantics.
src/hotspot/share/gc/shenandoah/shenandoahHeap.inline.hpp line 394:
> 392: }
> 393:
> 394: inline bool ShenandoahHeap::is_not_in_active_young_collection(oop obj) const {
This sounds to me more like: `is_in_old_when_active_young()`.
The "not" is the name makes the negative of its semantics suspect. Think of the sense of what one might believe if one identified the negation with `is_in_active_young_collection()` which I'd take to be `active_generation()->is_young() && is_in_young(obj)`, rather than the actual negation of the condition which would be: `!active_generation()->is_young() || !is_in_old(obj)`.
-------------
PR Review: https://git.openjdk.org/shenandoah/pull/517#pullrequestreview-2376364492
PR Review Comment: https://git.openjdk.org/shenandoah/pull/517#discussion_r1805481523
PR Review Comment: https://git.openjdk.org/shenandoah/pull/517#discussion_r1805480760
More information about the shenandoah-dev
mailing list