RFR: 8324553: Shenandoah: Move periodic tasks closer to their collaborators

Aleksey Shipilev shade at openjdk.org
Tue Jan 23 19:57:33 UTC 2024


On Tue, 23 Jan 2024 17:56:19 GMT, William Kemper <wkemper at openjdk.org> wrote:

> This is a refactoring to move Shenandoah's periodic tasks from the control thread to their respective dependencies: the pacer and monitoring support. This change will facilitate reviews of the upcoming changes to isolate the generational mode changes.

I need to look at it with fresh eyes tomorrow, but a couple of stylistic comments now:

src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp line 791:

> 789:   // update costs on slow path.
> 790:   monitoring_support()->notify_heap_changed();
> 791: 

Drop the new line.

src/hotspot/share/gc/shenandoah/shenandoahMonitoringSupport.cpp line 149:

> 147: void ShenandoahPeriodicCountersUpdate::set_forced_counters_update(bool value) {
> 148:   _force_counters_update.set_cond(value);
> 149: }

Newline is missing.

src/hotspot/share/gc/shenandoah/shenandoahMonitoringSupport.hpp line 69:

> 67: 
> 68:   ShenandoahHeapRegionCounters* _heap_region_counters;
> 69:   ShenandoahPeriodicCountersUpdate _counters_update;

`ShenandoahPeriodicCountersUpdateTask _counters_update_task;`?

src/hotspot/share/gc/shenandoah/shenandoahPacer.hpp line 47:

> 45:   void task() override;
> 46: private:
> 47:   ShenandoahPacer* _pacer;

Move this declaration at the beginning and make `_pacer` const, while you are at it?

src/hotspot/share/gc/shenandoah/shenandoahPacer.hpp line 67:

> 65:   Monitor* _wait_monitor;
> 66:   ShenandoahSharedFlag _need_notify_waiters;
> 67:   ShenandoahPeriodicPacerNotify _notify_waiters;

Let's call this `ShenandoahPeriodicPacerNotifyTask _notify_waiters_task`.

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

PR Review: https://git.openjdk.org/jdk/pull/17540#pullrequestreview-1839776983
PR Review Comment: https://git.openjdk.org/jdk/pull/17540#discussion_r1463919351
PR Review Comment: https://git.openjdk.org/jdk/pull/17540#discussion_r1463912502
PR Review Comment: https://git.openjdk.org/jdk/pull/17540#discussion_r1463916335
PR Review Comment: https://git.openjdk.org/jdk/pull/17540#discussion_r1463901023
PR Review Comment: https://git.openjdk.org/jdk/pull/17540#discussion_r1463901941


More information about the hotspot-gc-dev mailing list