RFR: 8300447: Parallel: Refactor PSPromotionManager::drain_stacks_depth

[Thomas Schatzl]

On Wed, 18 Jan 2023 08:56:52 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

> Simple refactoring based on the counterpart in G1.
> 
> Test: hotspot_gc

Changes requested by tschatzl (Reviewer).

src/hotspot/share/gc/parallel/psPromotionManager.cpp line 231:

> 229: void PSPromotionManager::drain_stacks_depth(bool totally_drain) {
> 230:   const uint threshold = totally_drain ? 0
> 231:                                        : _target_stack_size;

Feel free to ignore: I would prefer to not spread very simple `?:` statements like this over multiple lines. This is imo less readable than keeping it in a single line (and `?:` statements should always be very small/short anyway).

src/hotspot/share/gc/parallel/psPromotionManager.cpp line 246:

> 244:       process_popped_location_depth(task);
> 245:     }
> 246:   } while (!tq->overflow_empty());

The other, similar loops like the ones in G1 (but also the ones for parallel full gc) all try to first feed the public queue from the overflow queue like (to avoid other threads from starving while this thread happily processes the overflow queue contents on its own):


do {
  while (tq->pop_overflow(task)) {
    if (!tq->try_push_to_taskqueue(task)) {
      process_popped_location_depth(task);
    }
  }

  while (tq->pop_local(task, threshold)) {
    process_popped_location_depth(task);
  }
} while (!tq->overflow_empty());

Is there a reason to not change the structure of this loop to that better one?

(I am also kind of surprised that this change has not been made yet)

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

PR: https://git.openjdk.org/jdk/pull/12063