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