RFR: 8372250: Merge PtrQueue into SATBMarkQueue

Kim Barrett kbarrett at openjdk.org
Wed Dec 10 21:45:33 UTC 2025


On Wed, 10 Dec 2025 18:26:16 GMT, Ben Taylor <btaylor at openjdk.org> wrote:

> This change merges `PtrQueue` into its only child class, `SATBMarkQueue`. The corresponding `PtrQueueSet` is merged with `SATBMarkQueueSet`, and `G1SATBMarkQueueSet` and `ShenandoahSATBMarkQueueSet` continue to inherit from that class.
> 
> There are few code modifications - most methods were moved directly from `PtrQueue` to `SATBMarkQueue`. The one meaningful change is to  preserve `SATBMarkQueue`'s call to `filter(queue);` before `flush_queue`. 
> 
> This change passes all tier1 jtreg tests with both Shenandoah and G1 in local testing.

This looks mostly good, with just a few minor nits.

I think there may be further simplifications and cleanups possible, but
looking for those can be done later, once this relatively clean merging change
is done.

[appropriate place is not commentable in github diff]
SATBMarkQueueSet::enqueue_completed_buffer should no longer be virtual.  (This
would have been caught if it had been changed to "override" once that was
permitted, but nobody got around to that.)

src/hotspot/share/gc/shared/satbMarkQueue.cpp line 50:

> 48: 
> 49: SATBMarkQueue::~SATBMarkQueue() {
> 50:     assert(_buf == nullptr, "queue must be flushed before delete");

Too much indentation.

src/hotspot/share/gc/shared/satbMarkQueue.cpp line 428:

> 426: void SATBMarkQueueSet::deallocate_buffer(BufferNode* node) {
> 427:   _allocator->release(node);
> 428: }

Missing newline?

src/hotspot/share/gc/shared/satbMarkQueue.hpp line 196:

> 194:   // Installs a new buffer into queue.
> 195:   void install_new_buffer(SATBMarkQueue& queue);
> 196: 

These helper functions hoisted from PtrQueueSet should be private rather than protected here.

src/hotspot/share/gc/shared/satbMarkQueue.hpp line 249:

> 247: 
> 248:   // A completed buffer is a buffer the mutator is finished with, and
> 249:   // is ready to be processed by the collector.  It need not be full.

This comment (copied from ptrQueueSet) seems misplaced here. Perhaps it should be part of the
class description?

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

Changes requested by kbarrett (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/28755#pullrequestreview-3564478203
PR Review Comment: https://git.openjdk.org/jdk/pull/28755#discussion_r2608208876
PR Review Comment: https://git.openjdk.org/jdk/pull/28755#discussion_r2608213644
PR Review Comment: https://git.openjdk.org/jdk/pull/28755#discussion_r2608282954
PR Review Comment: https://git.openjdk.org/jdk/pull/28755#discussion_r2608246180


More information about the hotspot-gc-dev mailing list