fix: Account shared promotions unconditionally to avoid lost budget

This commit is contained in:
Xiaolong Peng 2026-06-03 09:12:14 -07:00
parent c9eb0e41db
commit e972be7793
4 changed files with 29 additions and 4 deletions

View File

@ -1039,10 +1039,12 @@ HeapWord* ShenandoahHeap::allocate_memory_work(ShenandoahAllocRequest& req, bool
old_generation()->configure_plab_for_current_thread(req);
} else if (req.is_promotion()) {
const size_t actual_size = req.actual_size() * HeapWordSize;
// Atomic reserve so concurrent GC workers can't over-expend the promotion budget without the lock.
if (!old_generation()->try_expend_promoted(actual_size)) {
log_debug(gc, plab)("Shared promotion of %zu bytes exceeded promotion reserve", actual_size);
}
// The object has already been promoted into old-gen, so account for it unconditionally.
// can_allocate() gated this promotion against the reserve under the heap lock, but the
// lock is released before we get here; a conditional reservation could fail under
// concurrent expends and silently lose accounting, leading to unbounded over-promotion.
log_debug(gc, plab)("Expend shared promotion of %zu bytes", actual_size);
old_generation()->expend_promoted(actual_size);
}
}
}

View File

@ -206,6 +206,10 @@ bool ShenandoahOldGeneration::try_expend_promoted(size_t increment) {
return false;
}
size_t ShenandoahOldGeneration::expend_promoted(size_t increment) {
return _promoted_expended.add_then_fetch(increment);
}
size_t ShenandoahOldGeneration::unexpend_promoted(size_t decrement) {
return _promoted_expended.sub_then_fetch(decrement);
}

View File

@ -113,8 +113,15 @@ public:
// Atomically reserve `increment` bytes of promotion budget. Returns true iff the full amount
// was reserved without exceeding the reserve. Lock-free: safe to call without the heap lock.
// Use this to gate a promotion decision before promoting.
bool try_expend_promoted(size_t increment);
// Unconditionally account `increment` bytes that have already been promoted into old-gen.
// Unlike try_expend_promoted, this never fails: the promotion has happened, so the budget
// must reflect it to stay truthful (it may briefly overshoot the reserve under concurrency).
// Lock-free: safe to call without the heap lock.
size_t expend_promoted(size_t increment);
// This is used to return unused memory from a retired promotion LAB
size_t unexpend_promoted(size_t decrement);

View File

@ -179,4 +179,16 @@ TEST_VM_F(ShenandoahOldGenerationTest, test_try_expend_promoted_should_increase_
EXPECT_EQ(expended_before + 128, expended_after) << "Should expend promotion";
}
TEST_VM_F(ShenandoahOldGenerationTest, test_expend_promoted_accounts_unconditionally) {
SKIP_IF_NOT_SHENANDOAH();
// try_expend_promoted gates a promotion decision and must fail past the reserve...
const size_t over = old->get_promoted_reserve() + 1;
size_t expended_before = old->get_promoted_expended();
EXPECT_FALSE(old->try_expend_promoted(over)) << "Reservation must fail past the reserve";
EXPECT_EQ(expended_before, old->get_promoted_expended()) << "Failed reservation must not change accounting";
// ...but expend_promoted accounts bytes already promoted unconditionally, even past the reserve.
EXPECT_EQ(expended_before + over, old->expend_promoted(over))
<< "Already-promoted bytes must be accounted even past the reserve";
}
#undef SKIP_IF_NOT_SHENANDOAH