RFR: 8369013: Shenandoah: passive mode should support enabling ShenandoahCardBarrier

William Kemper wkemper at openjdk.org
Tue Oct 28 17:33:42 UTC 2025


On Fri, 24 Oct 2025 00:18:03 GMT, Rui Li <duke at openjdk.org> wrote:

> Add card barriers to passive mode to test out the price of card barriers. 
> 
> How this change is implemented is to instantiate the old region in passive mode - old region owns the card table so this would minimize the code change with a bit price of native memory. It does sound weird to have old gen in passive mode, but since passive mode is a just diagnostic mode, we'll go with it for the cleanliness of the change.

This turned out to be simpler than expected. Nice work. I suggest some small changes.

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

> 45: 
> 46:   static ShenandoahGenerationalHeap* heap() {
> 47:     shenandoah_assert_generational();

Can we put these assertions back? I don't see any code in the PR that would invalidate these assertions. I also don't see any changes that would instantiate the generational heap for non generational mode, so if there _is_ code trying to use the generational heap, its behavior will be undefined.

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

> 253:   // for the purpose of having card table.
> 254:   if (ShenandoahCardBarrier && !(mode()->is_generational())) {
> 255:     _generation_sizer.heap_size_changed(max_capacity());

I think we could simplify here by not using the `_generation_sizer`. It should be fine to pass `max_capacity()` instead of `max_capacity_old` here.

src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp line 533:

> 531: 
> 532:   ShenandoahOldGeneration*   old_generation()    const {
> 533:     assert(mode()->is_generational(), "Old generation requires generational mode");

Can we assert that `ShenandoahCardBarrier` is on instead of removing this assertion?

src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp line 566:

> 564:   // For exporting to SA
> 565:   int                          _log_min_obj_alignment_in_bytes;
> 566:   ShenandoahGenerationSizer     _generation_sizer;

I don't think we need this here.

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

Changes requested by wkemper (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/27966#pullrequestreview-3378765228
PR Review Comment: https://git.openjdk.org/jdk/pull/27966#discussion_r2461936571
PR Review Comment: https://git.openjdk.org/jdk/pull/27966#discussion_r2461923434
PR Review Comment: https://git.openjdk.org/jdk/pull/27966#discussion_r2461925051
PR Review Comment: https://git.openjdk.org/jdk/pull/27966#discussion_r2461928806


More information about the hotspot-gc-dev mailing list