RFR: 8329658: Serial: Refactor ContiguousSpace::_next_compaction_space [v4]
Thomas Schatzl
tschatzl at openjdk.org
Fri Apr 12 07:34:44 UTC 2024
On Tue, 9 Apr 2024 14:05:25 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:
>> IMO, I think it is good to say: if the to-space is not empty, we need to deal with it, regardless whether it is failed promotion. Even only the failed promotion causes non empty to-space.
>>
>> What do you think about it?
>
> I have no preference regarding using a local-var or not.
>
> (My understanding is that non-empty-to-space == promotion-failure; if that's not true, I wanna refresh my knowledge on that.)
That is true, but since the only way this can happen is during promotion failure, the additional local var acts as a documentation about the expected situation this can happen. It isn't like that that `heap->young_gen()->to()->is_empty()` is lost because of that.
Also it's a statement with a triple indirection, so it would be nice to factor it out, whatever the name is, e.g.
bool to_space_empty = heap->young_gen()->to()->is_empty();
if (to_space_empty) {
or as suggested
bool promotion_failed = heap->young_gen()->to()->is_empty();
if (promotion_failed) {
seems better to me. I'll let the decision be up to you.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18666#discussion_r1562148632
More information about the hotspot-gc-dev
mailing list