From 05cd2d948c7fca1315a3f0e2d2646a30869cff23 Mon Sep 17 00:00:00 2001 From: Thomas Schatzl Date: Wed, 24 Jun 2026 08:50:49 +0000 Subject: [PATCH] 8381128: G1: Tighten accesses to TAMS/TARS Co-authored-by: Ivan Walulya Reviewed-by: iwalulya, ayang --- src/hotspot/share/gc/g1/g1CollectedHeap.cpp | 12 +- src/hotspot/share/gc/g1/g1CollectedHeap.hpp | 2 +- src/hotspot/share/gc/g1/g1CollectionSet.cpp | 2 +- .../share/gc/g1/g1CollectorState.inline.hpp | 11 +- src/hotspot/share/gc/g1/g1ConcurrentMark.cpp | 129 +++++++++++++----- src/hotspot/share/gc/g1/g1ConcurrentMark.hpp | 67 ++++++--- .../share/gc/g1/g1ConcurrentMark.inline.hpp | 74 +++++++++- .../gc/g1/g1ConcurrentMarkRemarkTasks.cpp | 2 +- .../share/gc/g1/g1ConcurrentMarkThread.hpp | 2 +- .../gc/g1/g1ConcurrentMarkThread.inline.hpp | 5 +- .../gc/g1/g1ConcurrentRebuildAndScrub.cpp | 7 +- src/hotspot/share/gc/g1/g1FullCollector.cpp | 2 +- .../share/gc/g1/g1FullGCResetMetadataTask.cpp | 3 +- src/hotspot/share/gc/g1/g1HeapRegion.cpp | 6 +- .../share/gc/g1/g1HeapRegion.inline.hpp | 4 - src/hotspot/share/gc/g1/g1HeapVerifier.cpp | 20 ++- src/hotspot/share/gc/g1/g1HeapVerifier.hpp | 5 +- src/hotspot/share/gc/g1/g1Policy.cpp | 4 +- src/hotspot/share/gc/g1/g1Policy.hpp | 5 +- .../share/gc/g1/g1RegionMarkStatsCache.hpp | 12 ++ src/hotspot/share/gc/g1/g1RemSet.cpp | 3 +- .../share/gc/g1/g1SATBMarkQueueSet.cpp | 11 +- src/hotspot/share/gc/g1/g1YoungCollector.cpp | 24 ++-- .../gc/g1/g1YoungGCPostEvacuateTasks.cpp | 23 ++-- src/hotspot/share/gc/shared/satbMarkQueue.cpp | 6 +- src/hotspot/share/gc/shared/satbMarkQueue.hpp | 6 +- .../g1/TestEagerReclaimHumongousRegions.java | 5 +- .../g1/TestVerificationInConcurrentCycle.java | 29 +++- .../pinnedobjs/TestDroppedRetainedTAMS.java | 9 +- 29 files changed, 367 insertions(+), 123 deletions(-) diff --git a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp index 233f993bb16..ebf7a1086fa 100644 --- a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp +++ b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp @@ -846,7 +846,7 @@ void G1CollectedHeap::prepare_heap_for_full_collection() { _hrm.remove_all_free_regions(); } -void G1CollectedHeap::verify_before_full_collection() { +void G1CollectedHeap::verify_before_full_collection(bool concurrent_cycle_aborted) { assert_used_and_recalculate_used_equal(this); if (!VerifyBeforeGC) { return; @@ -856,7 +856,7 @@ void G1CollectedHeap::verify_before_full_collection() { } _verifier->verify_region_sets_optional(); _verifier->verify_before_gc(); - _verifier->verify_bitmap_clear(true /* above_tams_only */); + _verifier->verify_bitmap_clear(true /* above_tams_only */, concurrent_cycle_aborted); } void G1CollectedHeap::prepare_for_mutator_after_full_collection(size_t allocation_word_size) { @@ -2880,6 +2880,7 @@ void G1CollectedHeap::free_region(G1HeapRegion* hr, G1FreeRegionList* free_list) // Reset region metadata to allow reuse. hr->hr_clear(true /* clear_space */); + concurrent_mark()->reset_region_marking_state(hr); _policy->remset_tracker()->update_at_free(hr); if (free_list != nullptr) { @@ -3192,6 +3193,9 @@ G1HeapRegion* G1CollectedHeap::new_gc_alloc_region(size_t word_size, G1HeapRegio // Synchronize with region attribute table. update_region_attr(new_alloc_region); } + + _cm->notify_new_region(new_alloc_region); + G1HeapRegionPrinter::alloc(new_alloc_region); return new_alloc_region; } @@ -3209,8 +3213,8 @@ void G1CollectedHeap::retire_gc_alloc_region(G1HeapRegion* alloc_region, _survivor.add_used_bytes(allocated_bytes); } - bool const during_im = collector_state()->is_in_concurrent_start_gc(); - if (during_im && allocated_bytes > 0) { + bool in_concurrent_start_gc = collector_state()->is_in_concurrent_start_gc(); + if (in_concurrent_start_gc && allocated_bytes > 0) { _cm->add_root_region(alloc_region); } G1HeapRegionPrinter::retire(alloc_region); diff --git a/src/hotspot/share/gc/g1/g1CollectedHeap.hpp b/src/hotspot/share/gc/g1/g1CollectedHeap.hpp index a0767d8fd0a..fc31878097b 100644 --- a/src/hotspot/share/gc/g1/g1CollectedHeap.hpp +++ b/src/hotspot/share/gc/g1/g1CollectedHeap.hpp @@ -524,7 +524,7 @@ private: // Internal helpers used during full GC to split it up to // increase readability. bool abort_concurrent_cycle(); - void verify_before_full_collection(); + void verify_before_full_collection(bool concurrent_cycle_aborted); void prepare_heap_for_full_collection(); void prepare_for_mutator_after_full_collection(size_t allocation_word_size); void abort_refinement(); diff --git a/src/hotspot/share/gc/g1/g1CollectionSet.cpp b/src/hotspot/share/gc/g1/g1CollectionSet.cpp index 14b5e321585..b0eb493120b 100644 --- a/src/hotspot/share/gc/g1/g1CollectionSet.cpp +++ b/src/hotspot/share/gc/g1/g1CollectionSet.cpp @@ -299,7 +299,7 @@ public: G1ConcurrentMark* cm = G1CollectedHeap::heap()->concurrent_mark(); _st->print_cr(" " HR_FORMAT ", TAMS: " PTR_FORMAT " PB: " PTR_FORMAT ", age: %4d", HR_FORMAT_PARAMS(r), - p2i(cm->top_at_mark_start(r)), + p2i(cm->top_at_mark_start_or_bottom(r)), p2i(r->parsable_bottom()), r->has_surv_rate_group() ? checked_cast(r->age_in_surv_rate_group()) : -1); return false; diff --git a/src/hotspot/share/gc/g1/g1CollectorState.inline.hpp b/src/hotspot/share/gc/g1/g1CollectorState.inline.hpp index b2d3dfcc489..b63d683bb63 100644 --- a/src/hotspot/share/gc/g1/g1CollectorState.inline.hpp +++ b/src/hotspot/share/gc/g1/g1CollectorState.inline.hpp @@ -77,19 +77,22 @@ inline bool G1CollectorState::initiate_conc_mark_if_possible() const { inline bool G1CollectorState::is_in_concurrent_cycle() const { G1ConcurrentMark* cm = G1CollectedHeap::heap()->concurrent_mark(); - return cm->is_in_concurrent_cycle(); + return cm->is_fully_initialized() && cm->is_in_concurrent_cycle(); } + inline bool G1CollectorState::is_in_marking() const { G1ConcurrentMark* cm = G1CollectedHeap::heap()->concurrent_mark(); - return cm->is_in_marking(); + return cm->is_fully_initialized() && cm->is_in_marking(); } + inline bool G1CollectorState::is_in_mark_or_rebuild() const { G1ConcurrentMark* cm = G1CollectedHeap::heap()->concurrent_mark(); - return is_in_marking() || cm->is_in_rebuild_or_scrub(); + return cm->is_fully_initialized() && cm->is_in_marking_or_rebuild(); } + inline bool G1CollectorState::is_in_reset_for_next_cycle() const { G1ConcurrentMark* cm = G1CollectedHeap::heap()->concurrent_mark(); - return cm->is_in_reset_for_next_cycle(); + return cm->is_fully_initialized() && cm->is_in_reset_for_next_cycle(); } inline void G1CollectorState::assert_is_young_pause(Pause type) { diff --git a/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp b/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp index 4afc7fa8ff1..11c93b092b1 100644 --- a/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp +++ b/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp @@ -449,7 +449,7 @@ G1ConcurrentMark::G1ConcurrentMark(G1CollectedHeap* g1h, _worker_id_offset(G1ConcRefinementThreads), // The refinement control thread does not refine cards, so it's just the worker threads. _max_num_tasks(MAX2(ConcGCThreads, ParallelGCThreads)), _num_active_tasks(0), // _num_active_tasks set in set_non_marking_state() - _tasks(nullptr), // _tasks set inside late_init() + _tasks(nullptr), _task_queues(new G1CMTaskQueueSet(_max_num_tasks)), _terminator(_max_num_tasks, _task_queues), _partial_array_state_manager(new PartialArrayStateManager(_max_num_tasks)), @@ -476,9 +476,10 @@ G1ConcurrentMark::G1ConcurrentMark(G1CollectedHeap* g1h, _num_concurrent_workers(0), _max_concurrent_workers(0), - _region_mark_stats(NEW_C_HEAP_ARRAY(G1RegionMarkStats, _g1h->max_num_regions(), mtGC)), - _top_at_mark_starts(NEW_C_HEAP_ARRAY(Atomic, _g1h->max_num_regions(), mtGC)), - _top_at_rebuild_starts(NEW_C_HEAP_ARRAY(Atomic, _g1h->max_num_regions(), mtGC)), + _is_region_mark_stats_cache_in_use(false), + _region_mark_stats(nullptr), + _top_at_mark_starts(nullptr), + _top_at_rebuild_starts(nullptr), _needs_remembered_set_rebuild(false) { assert(G1CGC_lock != nullptr, "CGC_lock must be initialized"); @@ -487,6 +488,8 @@ G1ConcurrentMark::G1ConcurrentMark(G1CollectedHeap* g1h, } void G1ConcurrentMark::fully_initialize() { + assert_at_safepoint(); + if (is_fully_initialized()) { return; } @@ -510,6 +513,10 @@ void G1ConcurrentMark::fully_initialize() { vm_exit_during_initialization("Failed to allocate initial concurrent mark overflow mark stack."); } + _region_mark_stats = NEW_C_HEAP_ARRAY(G1RegionMarkStats, _g1h->max_num_regions(), mtGC); + _top_at_mark_starts = NEW_C_HEAP_ARRAY(Atomic, _g1h->max_num_regions(), mtGC); + _top_at_rebuild_starts = NEW_C_HEAP_ARRAY(Atomic, _g1h->max_num_regions(), mtGC); + _tasks = NEW_C_HEAP_ARRAY(G1CMTask*, _max_num_tasks, mtGC); // so that the assertion in MarkingTaskQueue::task_queue doesn't fail @@ -527,22 +534,21 @@ void G1ConcurrentMark::fully_initialize() { for (uint i = 0; i < max_num_regions; i++) { ::new (&_top_at_mark_starts[i]) Atomic(_g1h->bottom_addr_for_region(i)); } - // Contrary to TAMS, the default value of _top_at_rebuild_starts needs to be null. ::new (_top_at_rebuild_starts) Atomic[max_num_regions]{}; reset_at_marking_complete(); } bool G1ConcurrentMark::is_in_concurrent_cycle() const { - return is_fully_initialized() ? _cm_thread->is_in_progress() : false; + return _cm_thread->is_in_progress(); } bool G1ConcurrentMark::is_in_marking() const { - return is_fully_initialized() ? cm_thread()->is_in_marking() : false; + return _cm_thread->is_in_marking(); } -bool G1ConcurrentMark::is_in_rebuild_or_scrub() const { - return cm_thread()->is_in_rebuild_or_scrub(); +bool G1ConcurrentMark::is_in_marking_or_rebuild() const { + return _cm_thread->is_in_marking_or_rebuild(); } bool G1ConcurrentMark::is_in_reset_for_next_cycle() const { @@ -559,8 +565,11 @@ G1ConcurrentMarkThread* G1ConcurrentMark::cm_thread() const { } void G1ConcurrentMark::reset() { + assert_fully_initialized(); + _has_aborted.store_relaxed(false); + _is_region_mark_stats_cache_in_use = true; reset_marking_for_restart(); // Reset all tasks, since different phases will use different number of active @@ -572,6 +581,9 @@ void G1ConcurrentMark::reset() { uint max_num_regions = _g1h->max_num_regions(); ::new (_top_at_rebuild_starts) Atomic[max_num_regions]{}; for (uint i = 0; i < max_num_regions; i++) { + // Do not update TAMS here. NoteStartOfMarkTask updates this in parallel in + // the pre-concurrent-start WorkerTask. + _top_at_rebuild_starts[i].store_relaxed(nullptr); _region_mark_stats[i].clear(); } @@ -579,11 +591,61 @@ void G1ConcurrentMark::reset() { _root_regions.reset(); } -void G1ConcurrentMark::clear_statistics(G1HeapRegion* r) { +void G1ConcurrentMark::assert_statistics_clear(G1HeapRegion* r) { + assert_fully_initialized(); +#ifdef ASSERT uint region_idx = r->hrm_index(); for (uint j = 0; j < _max_num_tasks; ++j) { - _tasks[j]->clear_mark_stats_cache(region_idx); + _tasks[j]->verify_no_mark_stats_for(r->hrm_index()); } + + assert(_top_at_rebuild_starts[region_idx].load_relaxed() == nullptr, "must be"); + + G1RegionMarkStats* s = &_region_mark_stats[region_idx]; + assert(s->incoming_refs() == 0, "must be"); + assert(s->live_words() == 0, "must be"); +#endif +} + +void G1ConcurrentMark::note_start_of_mark_for_region(G1HeapRegion* r) { + assert_at_safepoint(); + assert_fully_initialized(); + if (r->is_old_or_humongous() && !r->is_collection_set_candidate() && !r->in_collection_set()) { + update_top_at_mark_start(r); + } else { + set_top_at_mark_start_to_bottom(r); + } +} + +void G1ConcurrentMark::notify_new_region(G1HeapRegion* r, size_t marked_live_bytes_below_tams) { + assert_at_safepoint(); + if (!is_fully_initialized()) { + return; + } + G1CollectorState* state = _g1h->collector_state(); + if (state->is_in_concurrent_start_gc()) { + update_top_at_mark_start(r); + set_live_bytes(r->hrm_index(), marked_live_bytes_below_tams); + } +} + +void G1ConcurrentMark::reset_region_marking_state(G1HeapRegion* r) { + assert_at_safepoint(); + if (!is_fully_initialized()) { + return; + } + uint region_idx = r->hrm_index(); + // Only need to clear the stats cache for the given region if we are using the cache. + if (_is_region_mark_stats_cache_in_use) { + for (uint j = 0; j < _max_num_tasks; ++j) { + _tasks[j]->clear_mark_stats_cache(region_idx); + } + } else { + for (uint j = 0; j < _max_num_tasks; ++j) { + _tasks[j]->verify_no_mark_stats_for(region_idx); + } + } + set_top_at_mark_start_to_bottom(r); _top_at_rebuild_starts[region_idx].store_relaxed(nullptr); _region_mark_stats[region_idx].clear(); } @@ -594,19 +656,11 @@ void G1ConcurrentMark::humongous_object_eagerly_reclaimed(G1HeapRegion* r) { // Need to clear mark bit of the humongous object. Doing this unconditionally is fine. mark_bitmap()->clear(r->bottom()); - - if (!_g1h->collector_state()->is_in_mark_or_rebuild()) { - return; - } - - // Clear any statistics about the region gathered so far. - _g1h->humongous_obj_regions_iterate(r, - [&] (G1HeapRegion* r) { - clear_statistics(r); - }); } void G1ConcurrentMark::reset_marking_for_restart() { + assert_fully_initialized(); + _global_mark_stack.set_empty(); // Expand the marking stack, if we have to and if we can. @@ -729,7 +783,6 @@ private: assert(_bitmap->get_next_marked_addr(r->bottom(), r->end()) == r->end(), "Should not have marked bits"); return r->bottom(); } - assert(_bitmap->get_next_marked_addr(_cm->top_at_mark_start(r), r->end()) == r->end(), "Should not have marked bits above tams"); } return r->end(); } @@ -776,8 +829,6 @@ private: } assert(cur >= end, "Must have completed iteration over the bitmap for region %u.", r->hrm_index()); - _cm->reset_top_at_mark_start(r); - return false; } }; @@ -877,11 +928,7 @@ class G1PreConcurrentStartTask::NoteStartOfMarkTask : public G1AbstractSubTask { NoteStartOfMarkHRClosure() : G1HeapRegionClosure(), _cm(G1CollectedHeap::heap()->concurrent_mark()) { } bool do_heap_region(G1HeapRegion* r) override { - if (r->is_old_or_humongous() && !r->is_collection_set_candidate() && !r->in_collection_set()) { - _cm->update_top_at_mark_start(r); - } else { - _cm->reset_top_at_mark_start(r); - } + _cm->note_start_of_mark_for_region(r); return false; } } _region_cl; @@ -1189,6 +1236,11 @@ void G1ConcurrentMark::add_root_region(G1HeapRegion* r) { root_regions()->add(top_at_mark_start(r), r->top()); } +void G1ConcurrentMark::add_root_region_set_bottom(G1HeapRegion* r) { + set_top_at_mark_start_to_bottom(r); + root_regions()->add(r->bottom(), r->top()); +} + bool G1ConcurrentMark::is_root_region(G1HeapRegion* r) { return root_regions()->contains(MemRegion(top_at_mark_start(r), r->top())); } @@ -1822,7 +1874,7 @@ void G1ConcurrentMark::finalize_marking() { print_stats(); } -void G1ConcurrentMark::flush_all_task_caches() { +void G1ConcurrentMark::flush_all_task_caches(bool ends_use_of_mark_cache) { size_t hits = 0; size_t misses = 0; for (uint i = 0; i < _max_num_tasks; i++) { @@ -1833,6 +1885,9 @@ void G1ConcurrentMark::flush_all_task_caches() { size_t sum = hits + misses; log_debug(gc, stats)("Mark stats cache hits %zu misses %zu ratio %1.3lf", hits, misses, percent_of(hits, sum)); + if (ends_use_of_mark_cache) { + _is_region_mark_stats_cache_in_use = false; + } } void G1ConcurrentMark::clear_bitmap_for_region(G1HeapRegion* hr) { @@ -1870,7 +1925,8 @@ G1HeapRegion* G1ConcurrentMark::claim_region(uint worker_id) { return curr_region; } else { assert(limit == bottom, - "The region limit should be at bottom"); + "The scan limit for region %u (%s) should be bottom but is " PTR_FORMAT, + curr_region->hrm_index(), curr_region->get_short_type_str(), p2i(limit)); // We return null and the caller should try calling // claim_region() again. return nullptr; @@ -1878,7 +1934,9 @@ G1HeapRegion* G1ConcurrentMark::claim_region(uint worker_id) { } else { // Read the finger again. HeapWord* next_finger = finger(); - assert(next_finger > local_finger, "The finger should have moved forward " PTR_FORMAT " " PTR_FORMAT, p2i(local_finger), p2i(next_finger)); + assert(next_finger > local_finger, + "The finger should have moved forward " PTR_FORMAT " " PTR_FORMAT, + p2i(local_finger), p2i(next_finger)); local_finger = next_finger; } } @@ -2003,6 +2061,7 @@ bool G1ConcurrentMark::concurrent_cycle_abort() { return false; } + flush_all_task_caches(); reset_marking_for_restart(); abort_marking_threads(); @@ -2457,6 +2516,12 @@ void G1CMTask::drain_satb_buffers() { decrease_limits(); } +#ifndef PRODUCT +void G1CMTask::verify_no_mark_stats_for(uint region_idx) { + _mark_stats_cache.verify_no_mark_stats_for(region_idx); +} +#endif + void G1CMTask::clear_mark_stats_cache(uint region_idx) { _mark_stats_cache.reset(region_idx); } diff --git a/src/hotspot/share/gc/g1/g1ConcurrentMark.hpp b/src/hotspot/share/gc/g1/g1ConcurrentMark.hpp index 2040916b8e7..f0071286e04 100644 --- a/src/hotspot/share/gc/g1/g1ConcurrentMark.hpp +++ b/src/hotspot/share/gc/g1/g1ConcurrentMark.hpp @@ -494,15 +494,18 @@ class G1ConcurrentMark : public CHeapObj { // true, periodically insert checks to see if this method should exit prematurely. void clear_bitmap(WorkerThreads* workers, bool may_yield); + // Records whether the region mark stats cache may contain entries due to marking activity, + // and the cache for freed regions needs to be cleared for those. + bool _is_region_mark_stats_cache_in_use; // Region statistics gathered during marking. G1RegionMarkStats* _region_mark_stats; - // Top pointer for each region at the start of marking. Must be valid for all committed - // regions. + // Top pointer for each region at the start of marking. Must be valid, i.e. be within + // [bottom, end] of a region for all committed regions. Atomic* _top_at_mark_starts; // Top pointer for each region at the start of the rebuild remembered set process // for regions which remembered sets need to be rebuilt. A null for a given region - // means that this region does not be scanned during the rebuilding remembered - // set phase at all. + // means that this region does not need to be scanned during the remembered set rebuild + // phase at all. Atomic* _top_at_rebuild_starts; // True when Remark pause selected regions for rebuilding. bool _needs_remembered_set_rebuild; @@ -512,27 +515,47 @@ class G1ConcurrentMark : public CHeapObj { // Concurrent cycle state queries. bool is_in_concurrent_cycle() const; bool is_in_marking() const; - bool is_in_rebuild_or_scrub() const; + bool is_in_marking_or_rebuild() const; bool is_in_reset_for_next_cycle() const; + void assert_fully_initialized() const { assert(is_fully_initialized(), "must be"); } + // The TAMS may be read and returns useful values related to the current concurrent marking. + // This is the case only during the concurrent cycle or the Concurrent Start pause. + inline bool tams_may_be_read() const; + // Update the TAMS for the given region to the current top. + inline void update_top_at_mark_start(G1HeapRegion* r); + // Reset the TAMS for the given region to bottom. + inline void set_top_at_mark_start_to_bottom(G1HeapRegion* r); + public: // To be called when an object is marked the first time, e.g. after a successful // mark_in_bitmap call. Updates various statistics data. void add_to_liveness(uint worker_id, oop const obj, size_t size); // Did the last marking find a live object between bottom and TAMS? - bool contains_live_object(uint region) const { return _region_mark_stats[region].live_words() != 0; } + bool contains_live_object(uint region) const; // Live bytes in the given region as determined by concurrent marking, i.e. the amount of // live bytes between bottom and TAMS. - size_t live_bytes(uint region) const { return _region_mark_stats[region].live_words() * HeapWordSize; } + size_t live_bytes(uint region) const; // Set live bytes for concurrent marking. - void set_live_bytes(uint region, size_t live_bytes) { _region_mark_stats[region]._live_words.store_relaxed(live_bytes / HeapWordSize); } + void set_live_bytes(uint region, size_t live_bytes); // Approximate number of incoming references found during marking. - size_t incoming_refs(uint region) const { return _region_mark_stats[region].incoming_refs(); } + size_t incoming_refs(uint region) const; - // Update the TAMS for the given region to the current top. - inline void update_top_at_mark_start(G1HeapRegion* r); - // Reset the TAMS for the given region to bottom of that region. - inline void reset_top_at_mark_start(G1HeapRegion* r); + void note_start_of_mark_for_region(G1HeapRegion* r); + inline void assert_top_at_mark_start_is_bottom(G1HeapRegion* r); + + // Returns the TAMS for the given region; outside of the concurrent cycle or Concurrent Start + // pause, always returns r->bottom(). + // Intended to be used for queries that are not allowed to fail at any time, but give a + // reasonable value, e.g. for logging to avoid having to do lots of check at every call site. + // Do not use for logic. + inline HeapWord* top_at_mark_start_or_bottom(const G1HeapRegion* r) const; + // Special method to return TAMS for verification purposes. During verification, if Full GC + // aborted a concurrent cycle, we need to use the TAMS data because the bitmap < TAMS may + // legitimately contain marks, however since we are in a Full GC tams_may_be_read() returns + // false. The other methods would return bottom(), which is wrong for verification. + inline HeapWord* top_at_mark_start_for_verification(const G1HeapRegion* r, + bool concurrent_cycle_aborted) const; inline HeapWord* top_at_mark_start(const G1HeapRegion* r) const; inline HeapWord* top_at_mark_start(uint region) const; @@ -551,10 +574,16 @@ public: uint max_num_tasks() const {return _max_num_tasks; } - // Clear statistics gathered during the concurrent cycle for the given region after - // it has been reclaimed. - void clear_statistics(G1HeapRegion* r); - // Notification for eagerly reclaimed regions to clean up. + void assert_statistics_clear(G1HeapRegion* r); + + // Notification for marking that a new region has been added to the heap. Updates the TAMS and + // live bytes for this region during a Concurrent Start pause. + void notify_new_region(G1HeapRegion* r, size_t marked_live_bytes_below_tams = 0); + + // Resets region marking state for the given region, i.e. TAMS, statistics, task metadata, + // etc. to initial state. + void reset_region_marking_state(G1HeapRegion* r); + // Notification for eagerly reclaimed regions to do extra clean up. void humongous_object_eagerly_reclaimed(G1HeapRegion* r); // Manipulation of the global mark stack. // The push and pop operations are used by tasks for transfers @@ -605,7 +634,7 @@ public: void reset(); // Moves all per-task cached data into global state. - void flush_all_task_caches(); + void flush_all_task_caches(bool ends_use_of_mark_cache = true); // Prepare internal data structures for the next mark cycle. This includes clearing // the next mark bitmap and some internal data structures. This method is intended // to be called concurrently to the mutator. It will yield to safepoint requests. @@ -632,6 +661,7 @@ public: void stop(); void add_root_region(G1HeapRegion* r); + void add_root_region_set_bottom(G1HeapRegion* r); bool is_root_region(G1HeapRegion* r); // Scan all the root regions concurrently and mark everything reachable from @@ -958,6 +988,7 @@ public: inline void inc_incoming_refs(oop const obj); + void verify_no_mark_stats_for(uint region_idx) PRODUCT_RETURN; // Clear (without flushing) the mark cache entry for the given region. void clear_mark_stats_cache(uint region_idx); // Evict the whole statistics cache into the global statistics. Returns the diff --git a/src/hotspot/share/gc/g1/g1ConcurrentMark.inline.hpp b/src/hotspot/share/gc/g1/g1ConcurrentMark.inline.hpp index 094f4dca994..ec6a486dc02 100644 --- a/src/hotspot/share/gc/g1/g1ConcurrentMark.inline.hpp +++ b/src/hotspot/share/gc/g1/g1ConcurrentMark.inline.hpp @@ -28,6 +28,7 @@ #include "gc/g1/g1ConcurrentMark.hpp" #include "gc/g1/g1CollectedHeap.inline.hpp" +#include "gc/g1/g1CollectorState.inline.hpp" #include "gc/g1/g1ConcurrentMarkBitMap.inline.hpp" #include "gc/g1/g1HeapRegion.hpp" #include "gc/g1/g1HeapRegionRemSet.inline.hpp" @@ -189,20 +190,69 @@ inline void G1CMTask::process_array_chunk(objArrayOop obj, size_t start, size_t } inline void G1ConcurrentMark::update_top_at_mark_start(G1HeapRegion* r) { + assert_fully_initialized(); + assert(_g1h->collector_state()->is_in_concurrent_start_gc(), "must be"); uint const region = r->hrm_index(); assert(region < _g1h->max_num_regions(), "Tried to access TAMS for region %u out of bounds", region); _top_at_mark_starts[region].store_relaxed(r->top()); } -inline void G1ConcurrentMark::reset_top_at_mark_start(G1HeapRegion* r) { +inline void G1ConcurrentMark::set_top_at_mark_start_to_bottom(G1HeapRegion* r) { + assert_fully_initialized(); _top_at_mark_starts[r->hrm_index()].store_relaxed(r->bottom()); } +inline void G1ConcurrentMark::assert_top_at_mark_start_is_bottom(G1HeapRegion* r) { + // Can not assert anything if not initialized. + if (!tams_may_be_read()) { + return; + } + HeapWord* local_top_at_mark_start = top_at_mark_start(r); + assert(local_top_at_mark_start == r->bottom(), + "must be, but tams for r %u (%s) is" PTR_FORMAT, + r->hrm_index(), r->get_short_type_str(), p2i(local_top_at_mark_start)); +} + +inline HeapWord* G1ConcurrentMark::top_at_mark_start_or_bottom(const G1HeapRegion* r) const { + if (!tams_may_be_read()) { + return r->bottom(); + } + return top_at_mark_start(r); +} + +inline HeapWord* G1ConcurrentMark::top_at_mark_start_for_verification(const G1HeapRegion* r, + bool concurrent_cycle_aborted) const { + if (!is_fully_initialized()) { + // We do not have TAMS data yet. + return r->bottom(); + } + if (tams_may_be_read()) { + // Normal case, we can read TAMS data and it is valid. + return top_at_mark_start(r); + } + if (concurrent_cycle_aborted) { + assert(_g1h->collector_state()->is_in_full_gc(), "Must be in Full GC if concurrent cycle has aborted"); + assert(r->hrm_index() < _g1h->max_num_regions(), + "Tried to access TAMS for region %u out of bounds", r->hrm_index()); + return _top_at_mark_starts[r->hrm_index()].load_relaxed(); + } + return r->bottom(); +} + +inline bool G1ConcurrentMark::tams_may_be_read() const { + // We need the TAMS to be valid even outside of actual marking for e.g. clearing the bitmap. + G1CollectorState* state = _g1h->collector_state(); + return is_fully_initialized() && + (state->is_in_concurrent_cycle() || state->is_in_concurrent_start_gc()); +} + inline HeapWord* G1ConcurrentMark::top_at_mark_start(const G1HeapRegion* r) const { return top_at_mark_start(r->hrm_index()); } inline HeapWord* G1ConcurrentMark::top_at_mark_start(uint region) const { + assert_fully_initialized(); + assert(tams_may_be_read(), "must be"); assert(region < _g1h->max_num_regions(), "Tried to access TARS for region %u out of bounds", region); return _top_at_mark_starts[region].load_relaxed(); } @@ -214,10 +264,12 @@ inline bool G1ConcurrentMark::obj_allocated_since_mark_start(oop obj) const { } inline HeapWord* G1ConcurrentMark::top_at_rebuild_start(G1HeapRegion* r) const { + assert_fully_initialized(); return _top_at_rebuild_starts[r->hrm_index()].load_relaxed(); } inline void G1ConcurrentMark::update_top_at_rebuild_start(G1HeapRegion* r) { + assert_fully_initialized(); assert(r->is_old() || r->is_humongous(), "precondition"); uint const region = r->hrm_index(); @@ -240,6 +292,26 @@ inline void G1ConcurrentMark::add_to_liveness(uint worker_id, oop const obj, siz task(worker_id)->update_liveness(obj, size); } +inline bool G1ConcurrentMark::contains_live_object(uint region) const { + assert_fully_initialized(); + return _region_mark_stats[region].live_words() != 0; +} + +inline size_t G1ConcurrentMark::live_bytes(uint region) const { + assert_fully_initialized(); + return _region_mark_stats[region].live_words() * HeapWordSize; +} + +inline void G1ConcurrentMark::set_live_bytes(uint region, size_t live_bytes) { + assert_fully_initialized(); + _region_mark_stats[region]._live_words.store_relaxed(live_bytes / HeapWordSize); +} + +inline size_t G1ConcurrentMark::incoming_refs(uint region) const { + assert_fully_initialized(); + return _region_mark_stats[region].incoming_refs(); +} + inline void G1CMTask::abort_marking_if_regular_check_fail() { if (!regular_clock_call()) { set_has_aborted(); diff --git a/src/hotspot/share/gc/g1/g1ConcurrentMarkRemarkTasks.cpp b/src/hotspot/share/gc/g1/g1ConcurrentMarkRemarkTasks.cpp index 3eda7200e25..02c1d2bf0d3 100644 --- a/src/hotspot/share/gc/g1/g1ConcurrentMarkRemarkTasks.cpp +++ b/src/hotspot/share/gc/g1/g1ConcurrentMarkRemarkTasks.cpp @@ -65,7 +65,7 @@ struct G1UpdateRegionLivenessAndSelectForRebuildTask::G1OnRegionClosure : public _freed_bytes += hr->used(); hr->set_containing_set(nullptr); hr->clear_both_card_tables(); - _cm->clear_statistics(hr); + _cm->assert_statistics_clear(hr); G1HeapRegionPrinter::mark_reclaim(hr); _g1h->concurrent_refine()->notify_region_reclaimed(hr); } diff --git a/src/hotspot/share/gc/g1/g1ConcurrentMarkThread.hpp b/src/hotspot/share/gc/g1/g1ConcurrentMarkThread.hpp index ad6062d0239..a22442c2b7f 100644 --- a/src/hotspot/share/gc/g1/g1ConcurrentMarkThread.hpp +++ b/src/hotspot/share/gc/g1/g1ConcurrentMarkThread.hpp @@ -115,7 +115,7 @@ class G1ConcurrentMarkThread: public ConcurrentGCThread { bool is_in_progress() const; bool is_in_marking() const; - bool is_in_rebuild_or_scrub() const; + bool is_in_marking_or_rebuild() const; bool is_in_reset_for_next_cycle() const; bool is_in_undo_cycle() const; diff --git a/src/hotspot/share/gc/g1/g1ConcurrentMarkThread.inline.hpp b/src/hotspot/share/gc/g1/g1ConcurrentMarkThread.inline.hpp index be2bc8e9e7a..bea6fe4e451 100644 --- a/src/hotspot/share/gc/g1/g1ConcurrentMarkThread.inline.hpp +++ b/src/hotspot/share/gc/g1/g1ConcurrentMarkThread.inline.hpp @@ -79,8 +79,9 @@ inline bool G1ConcurrentMarkThread::is_in_marking() const { return state() == FullCycleMarking; } -inline bool G1ConcurrentMarkThread::is_in_rebuild_or_scrub() const { - return state() == FullCycleRebuildOrScrub; +inline bool G1ConcurrentMarkThread::is_in_marking_or_rebuild() const { + ServiceState st = state(); + return st == FullCycleMarking || st == FullCycleRebuildOrScrub; } inline bool G1ConcurrentMarkThread::is_in_reset_for_next_cycle() const { diff --git a/src/hotspot/share/gc/g1/g1ConcurrentRebuildAndScrub.cpp b/src/hotspot/share/gc/g1/g1ConcurrentRebuildAndScrub.cpp index cd560a41333..5b652f096a7 100644 --- a/src/hotspot/share/gc/g1/g1ConcurrentRebuildAndScrub.cpp +++ b/src/hotspot/share/gc/g1/g1ConcurrentRebuildAndScrub.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2022, 2026, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -180,7 +180,10 @@ class G1RebuildRSAndScrubTask : public WorkerTask { assert(should_rebuild_or_scrub(hr), "must be"); log_trace(gc, marking)("Scrub and rebuild region: " HR_FORMAT " pb: " PTR_FORMAT " TARS: " PTR_FORMAT " TAMS: " PTR_FORMAT, - HR_FORMAT_PARAMS(hr), p2i(pb), p2i(_cm->top_at_rebuild_start(hr)), p2i(_cm->top_at_mark_start(hr))); + HR_FORMAT_PARAMS(hr), + p2i(pb), + p2i(_cm->top_at_rebuild_start(hr)), + p2i(_cm->top_at_mark_start_or_bottom(hr))); { // Step 1: Scan the given region from bottom to parsable_bottom. diff --git a/src/hotspot/share/gc/g1/g1FullCollector.cpp b/src/hotspot/share/gc/g1/g1FullCollector.cpp index cf153226920..c5af4a8220b 100644 --- a/src/hotspot/share/gc/g1/g1FullCollector.cpp +++ b/src/hotspot/share/gc/g1/g1FullCollector.cpp @@ -192,7 +192,7 @@ void G1FullCollector::prepare_collection() { // Verification needs the bitmap, so we should clear the bitmap only later. bool in_concurrent_cycle = _heap->abort_concurrent_cycle(); - _heap->verify_before_full_collection(); + _heap->verify_before_full_collection(in_concurrent_cycle); if (in_concurrent_cycle) { GCTraceTime(Debug, gc) debug("Clear Bitmap"); _heap->concurrent_mark()->clear_bitmap(_heap->workers()); diff --git a/src/hotspot/share/gc/g1/g1FullGCResetMetadataTask.cpp b/src/hotspot/share/gc/g1/g1FullGCResetMetadataTask.cpp index c8fba3459aa..310cc4297c6 100644 --- a/src/hotspot/share/gc/g1/g1FullGCResetMetadataTask.cpp +++ b/src/hotspot/share/gc/g1/g1FullGCResetMetadataTask.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2023, 2026, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -35,6 +35,7 @@ void G1FullGCResetMetadataTask::G1ResetMetadataClosure::reset_region_metadata(G1 "Non-humongous regions must not have cset group"); hr->rem_set()->clear(); hr->clear_both_card_tables(); + _g1h->concurrent_mark()->reset_region_marking_state(hr); } bool G1FullGCResetMetadataTask::G1ResetMetadataClosure::do_heap_region(G1HeapRegion* hr) { diff --git a/src/hotspot/share/gc/g1/g1HeapRegion.cpp b/src/hotspot/share/gc/g1/g1HeapRegion.cpp index 2052a3ce156..810bd4df2ee 100644 --- a/src/hotspot/share/gc/g1/g1HeapRegion.cpp +++ b/src/hotspot/share/gc/g1/g1HeapRegion.cpp @@ -129,8 +129,6 @@ void G1HeapRegion::hr_clear(bool clear_space) { rem_set()->clear(); - G1CollectedHeap::heap()->concurrent_mark()->reset_top_at_mark_start(this); - _parsable_bottom.store_relaxed(bottom()); _garbage_bytes.store_relaxed(0); _incoming_refs = 0; @@ -439,7 +437,9 @@ void G1HeapRegion::print_on(outputStream* st) const { } G1ConcurrentMark* cm = G1CollectedHeap::heap()->concurrent_mark(); st->print("|TAMS " PTR_FORMAT "| PB " PTR_FORMAT "| %-9s ", - p2i(cm->top_at_mark_start(this)), p2i(parsable_bottom_acquire()), rem_set()->get_state_str()); + p2i(cm->top_at_mark_start_or_bottom(this)), + p2i(parsable_bottom_acquire()), + rem_set()->get_state_str()); if (UseNUMA) { G1NUMA* numa = G1NUMA::numa(); if (node_index() < numa->num_active_nodes()) { diff --git a/src/hotspot/share/gc/g1/g1HeapRegion.inline.hpp b/src/hotspot/share/gc/g1/g1HeapRegion.inline.hpp index 619aef35a9a..87597d450cb 100644 --- a/src/hotspot/share/gc/g1/g1HeapRegion.inline.hpp +++ b/src/hotspot/share/gc/g1/g1HeapRegion.inline.hpp @@ -152,10 +152,6 @@ inline void G1HeapRegion::reset_skip_compacting_after_full_gc() { } inline void G1HeapRegion::reset_after_full_gc_common() { - // After a full gc the mark information in a movable region is invalid. Reset marking - // information. - G1CollectedHeap::heap()->concurrent_mark()->reset_top_at_mark_start(this); - // Everything above bottom() is parsable and live. reset_parsable_bottom(); diff --git a/src/hotspot/share/gc/g1/g1HeapVerifier.cpp b/src/hotspot/share/gc/g1/g1HeapVerifier.cpp index 304722c13a1..7b518379a73 100644 --- a/src/hotspot/share/gc/g1/g1HeapVerifier.cpp +++ b/src/hotspot/share/gc/g1/g1HeapVerifier.cpp @@ -499,12 +499,14 @@ void G1HeapVerifier::verify_marking_state() { // Verify TAMSes, bitmaps and liveness statistics. // // - if part of marking: TAMS != bottom, liveness == 0, bitmap clear - // - if evacuation failed + part of marking: TAMS != bottom, liveness != 0, bitmap has at least on object set (corresponding to liveness) + // - if evacuation failed + part of marking: TAMS != bottom, liveness != 0, bitmap has at least one + // object set (corresponding to liveness) // - if not part of marking: TAMS == bottom, liveness == 0, bitmap clear; must be in root region // To compare liveness recorded in G1ConcurrentMark and actual we need to flush the - // cache. - G1CollectedHeap::heap()->concurrent_mark()->flush_all_task_caches(); + // cache. Do not signal end of use of the mark stats cache as this flush is only to + // make verification work. Further concurrent marking continues to need these values. + G1CollectedHeap::heap()->concurrent_mark()->flush_all_task_caches(false /* ends_use_of_mark_cache */); G1VerifyRegionMarkingStateClosure cl; _g1h->heap_region_iterate(&cl); @@ -532,28 +534,32 @@ void G1HeapVerifier::verify_after_gc() { verify_card_tables_in_sync(); } -void G1HeapVerifier::verify_bitmap_clear(bool from_tams) { +void G1HeapVerifier::verify_bitmap_clear(bool from_tams, bool concurrent_cycle_aborted) { if (!G1VerifyBitmaps) { return; } class G1VerifyBitmapClear : public G1HeapRegionClosure { bool _from_tams; + bool _concurrent_cycle_aborted; public: - G1VerifyBitmapClear(bool from_tams) : _from_tams(from_tams) { } + G1VerifyBitmapClear(bool from_tams, bool concurrent_cycle_aborted) : + _from_tams(from_tams), _concurrent_cycle_aborted(concurrent_cycle_aborted) { } virtual bool do_heap_region(G1HeapRegion* r) { G1ConcurrentMark* cm = G1CollectedHeap::heap()->concurrent_mark(); G1CMBitMap* bitmap = cm->mark_bitmap(); - HeapWord* start = _from_tams ? cm->top_at_mark_start(r) : r->bottom(); + HeapWord* start = _from_tams + ? cm->top_at_mark_start_for_verification(r, _concurrent_cycle_aborted) + : r->bottom(); HeapWord* mark = bitmap->get_next_marked_addr(start, r->end()); guarantee(mark == r->end(), "Found mark at " PTR_FORMAT " in region %u from start " PTR_FORMAT, p2i(mark), r->hrm_index(), p2i(start)); return false; } - } cl(from_tams); + } cl(from_tams, concurrent_cycle_aborted); G1CollectedHeap::heap()->heap_region_iterate(&cl); } diff --git a/src/hotspot/share/gc/g1/g1HeapVerifier.hpp b/src/hotspot/share/gc/g1/g1HeapVerifier.hpp index 55f6646563f..05a89a524d6 100644 --- a/src/hotspot/share/gc/g1/g1HeapVerifier.hpp +++ b/src/hotspot/share/gc/g1/g1HeapVerifier.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2026, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -73,7 +73,8 @@ public: // Verify that marking state is set up correctly after a concurrent start pause. void verify_marking_state(); - void verify_bitmap_clear(bool above_tams_only); + void verify_bitmap_clear(bool above_tams_only, + bool concurrent_cycle_aborted = false); // Do sanity check on the contents of the in-cset fast test table. bool check_region_attr_table() PRODUCT_RETURN_( return true; ); diff --git a/src/hotspot/share/gc/g1/g1Policy.cpp b/src/hotspot/share/gc/g1/g1Policy.cpp index 04afd262dd4..6aeabb9ac9b 100644 --- a/src/hotspot/share/gc/g1/g1Policy.cpp +++ b/src/hotspot/share/gc/g1/g1Policy.cpp @@ -672,8 +672,8 @@ void G1Policy::record_dirtying_stats(double last_mutator_start_dirty_ms, _to_collection_set_cards = next_to_collection_set_cards; } -bool G1Policy::should_retain_evac_failed_region(uint index) const { - size_t live_bytes = _g1h->region_at(index)->live_bytes(); +bool G1Policy::should_retain_evac_failed_region(G1HeapRegion* r) const { + size_t live_bytes = r->live_bytes(); size_t threshold = G1RetainRegionLiveThresholdPercent * G1HeapRegion::GrainBytes / 100; return live_bytes < threshold; } diff --git a/src/hotspot/share/gc/g1/g1Policy.hpp b/src/hotspot/share/gc/g1/g1Policy.hpp index 3cfd54c8c94..b661daa9a3e 100644 --- a/src/hotspot/share/gc/g1/g1Policy.hpp +++ b/src/hotspot/share/gc/g1/g1Policy.hpp @@ -389,10 +389,7 @@ public: size_t next_pending_cards_from_gc, size_t next_to_collection_set_cards); - bool should_retain_evac_failed_region(G1HeapRegion* r) const { - return should_retain_evac_failed_region(r->hrm_index()); - } - bool should_retain_evac_failed_region(uint index) const; + bool should_retain_evac_failed_region(G1HeapRegion* r) const; private: // diff --git a/src/hotspot/share/gc/g1/g1RegionMarkStatsCache.hpp b/src/hotspot/share/gc/g1/g1RegionMarkStatsCache.hpp index df76147f4b1..4db2dbd2287 100644 --- a/src/hotspot/share/gc/g1/g1RegionMarkStatsCache.hpp +++ b/src/hotspot/share/gc/g1/g1RegionMarkStatsCache.hpp @@ -44,6 +44,8 @@ struct G1RegionMarkStats { Atomic _live_words; Atomic _incoming_refs; + G1RegionMarkStats() : _live_words(0), _incoming_refs(0) { } + // Clear all members. void clear() { _live_words.store_relaxed(0); @@ -121,6 +123,16 @@ public: cur->_stats._live_words.store_relaxed(cur->_stats.live_words() + live_words); } + void verify_no_mark_stats_for(uint region_idx) { + uint const cache_idx = hash(region_idx); + G1RegionMarkStatsCacheEntry* const cur = &_cache[cache_idx]; + if (cur->_region_idx != region_idx) { + return; + } + assert(cur->_stats.incoming_refs() == 0, "must be"); + assert(cur->_stats.live_words() == 0, "must be"); + } + void inc_incoming_refs(uint region_idx) { G1RegionMarkStatsCacheEntry* const cur = find_for_add(region_idx); // This method is only ever called single-threaded, so we do not need atomic diff --git a/src/hotspot/share/gc/g1/g1RemSet.cpp b/src/hotspot/share/gc/g1/g1RemSet.cpp index 608f12e3859..bcb50dcc98f 100644 --- a/src/hotspot/share/gc/g1/g1RemSet.cpp +++ b/src/hotspot/share/gc/g1/g1RemSet.cpp @@ -1061,11 +1061,10 @@ class G1MergeHeapRootsTask : public WorkerTask { // so the bitmap for the regions in the collection set must be cleared if not already. if (should_clear_region(hr)) { _g1h->clear_bitmap_for_region(hr); - _g1h->concurrent_mark()->reset_top_at_mark_start(hr); } else { assert_bitmap_clear(hr, _g1h->concurrent_mark()->mark_bitmap()); } - _g1h->concurrent_mark()->clear_statistics(hr); + _g1h->concurrent_mark()->reset_region_marking_state(hr); _scan_state->add_all_dirty_region(hr->hrm_index()); return false; } diff --git a/src/hotspot/share/gc/g1/g1SATBMarkQueueSet.cpp b/src/hotspot/share/gc/g1/g1SATBMarkQueueSet.cpp index 7d197c37158..7553936bb26 100644 --- a/src/hotspot/share/gc/g1/g1SATBMarkQueueSet.cpp +++ b/src/hotspot/share/gc/g1/g1SATBMarkQueueSet.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2018, 2026, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -114,5 +114,12 @@ public: }; void G1SATBMarkQueueSet::filter(SATBMarkQueue& queue) { - apply_filter(G1SATBMarkQueueFilterFn(), queue); + G1CollectedHeap* g1h = G1CollectedHeap::heap(); + if (g1h->collector_state()->is_in_marking()) { + apply_filter(G1SATBMarkQueueFilterFn(), queue); + } else { + // is_in_marking() covers both the concurrent marking and the Remark pause. Outside + // of that, there can be no entry that requires SATB marking. + queue.set_empty(); + } } diff --git a/src/hotspot/share/gc/g1/g1YoungCollector.cpp b/src/hotspot/share/gc/g1/g1YoungCollector.cpp index edfe97d04d6..ec83d8a27d3 100644 --- a/src/hotspot/share/gc/g1/g1YoungCollector.cpp +++ b/src/hotspot/share/gc/g1/g1YoungCollector.cpp @@ -362,13 +362,14 @@ class G1PrepareEvacuationTask : public WorkerTask { // There is no difference between scanning cards covering an effectively // dead humongous object vs. some other objects in reallocated regions. // - // TAMSes are only reset after completing the entire mark cycle, during - // bitmap clearing. It is worth to not wait until then, and allow reclamation - // outside of actual (concurrent) SATB marking. + // TAMSes are only reset in the Concurrent Start pause and when they are + // reclaimed/freed. It is worth to not wait for TAMS updates until either + // of these conditions applies and allow reclamation as much as possible. // This also applies to the concurrent start pause - we only set - // mark_in_progress() at the end of that GC: no mutator is running that can + // is_in_marking() at the end of that GC: no mutator is running that can // sneakily install a new reference to the potentially reclaimed humongous // object. + // // During the concurrent start pause the situation described above where we // miss a reference can not happen. No mutator is modifying the object // graph to install such an overlooked reference. @@ -376,12 +377,15 @@ class G1PrepareEvacuationTask : public WorkerTask { // After the pause, having reclaimed h, obviously the mutator can't fetch // the reference from h any more. if (!obj->is_typeArray()) { - // All regions that were allocated before marking have a TAMS != bottom. - bool allocated_before_mark_start = region->bottom() != _g1h->concurrent_mark()->top_at_mark_start(region); bool mark_in_progress = _g1h->collector_state()->is_in_marking(); - - if (allocated_before_mark_start && mark_in_progress) { - return false; + // top_at_mark_start() will assert outside of marking, so check first. + if (mark_in_progress) { + // All regions that were allocated before marking have a TAMS != bottom. + G1ConcurrentMark* cm = _g1h->concurrent_mark(); + bool allocated_before_mark_start = region->bottom() != cm->top_at_mark_start(region); + if (allocated_before_mark_start) { + return false; + } } } return _g1h->is_potential_eager_reclaim_candidate(region); @@ -1028,7 +1032,7 @@ void G1YoungCollector::enqueue_candidates_as_root_regions() { G1CollectionSetCandidates* candidates = collection_set()->candidates(); candidates->iterate_regions([&] (G1HeapRegion* r) { - _g1h->concurrent_mark()->add_root_region(r); + _g1h->concurrent_mark()->add_root_region_set_bottom(r); }); } diff --git a/src/hotspot/share/gc/g1/g1YoungGCPostEvacuateTasks.cpp b/src/hotspot/share/gc/g1/g1YoungGCPostEvacuateTasks.cpp index cc9c4b10202..97378d0542e 100644 --- a/src/hotspot/share/gc/g1/g1YoungGCPostEvacuateTasks.cpp +++ b/src/hotspot/share/gc/g1/g1YoungGCPostEvacuateTasks.cpp @@ -394,8 +394,13 @@ public: oop obj = cast_to_oop(r->bottom()); { ResourceMark rm; - bool allocated_after_mark_start = r->bottom() == _g1h->concurrent_mark()->top_at_mark_start(r); bool mark_in_progress = _g1h->collector_state()->is_in_marking(); + bool allocated_after_mark_start = false; + if (mark_in_progress) { + // top_at_mark_start() will assert if we are not in marking, so check first. + allocated_after_mark_start = r->bottom() == _g1h->concurrent_mark()->top_at_mark_start(r); + } + guarantee(obj->is_typeArray() || (allocated_after_mark_start || !mark_in_progress), "Only eagerly reclaiming primitive arrays is supported, other humongous objects only if allocated after mark start, but the object " PTR_FORMAT " (%s) is not (mark %d allocated after mark: %d).", @@ -498,20 +503,22 @@ class G1PostEvacuateCollectionSetCleanupTask2::ProcessEvacuationFailedRegionsTas G1CollectedHeap* g1h = G1CollectedHeap::heap(); G1ConcurrentMark* cm = g1h->concurrent_mark(); - // Concurrent mark does not mark through regions that we retain (they are root - // regions wrt to marking), so we must clear their mark data (tams, bitmap, ...) - // set eagerly or during evacuation failure. + // Retained regions are root regions for marking, so we must clear their mark data + // (tams, bitmap, ...). Outside of Concurrent Start GC we must always clear the mark data + // for the next GC. bool clear_mark_data = !g1h->collector_state()->is_in_concurrent_start_gc() || g1h->policy()->should_retain_evac_failed_region(r); if (clear_mark_data) { g1h->clear_bitmap_for_region(r); + // Must be because this is a region that should not have been selected to + // be marked through. + cm->assert_top_at_mark_start_is_bottom(r); } else { // This evacuation failed region is going to be marked through. Update mark data. - cm->update_top_at_mark_start(r); - cm->set_live_bytes(r->hrm_index(), r->live_bytes()); - assert(cm->mark_bitmap()->get_next_marked_addr(r->bottom(), cm->top_at_mark_start(r)) != cm->top_at_mark_start(r), - "Marks must be on bitmap for region %u", r->hrm_index()); + // Since we have some marked live data information, pass that too. + cm->assert_statistics_clear(r); + cm->notify_new_region(r, r->live_bytes()); } return false; } diff --git a/src/hotspot/share/gc/shared/satbMarkQueue.cpp b/src/hotspot/share/gc/shared/satbMarkQueue.cpp index 63496f2eb25..a1bd4e5a9c8 100644 --- a/src/hotspot/share/gc/shared/satbMarkQueue.cpp +++ b/src/hotspot/share/gc/shared/satbMarkQueue.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2001, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2001, 2026, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -199,7 +199,7 @@ void SATBMarkQueueSet::set_active_all_threads(bool active, bool expected_active) if (_active) { assert(queue.is_empty(), "queues should be empty when activated"); } else { - queue.set_index(queue.current_capacity()); + queue.set_empty(); } queue.set_active(_active); } @@ -363,7 +363,7 @@ size_t SATBMarkQueue::current_capacity() const { } void SATBMarkQueueSet::reset_queue(SATBMarkQueue& queue) { - queue.set_index(queue.current_capacity()); + queue.set_empty(); } void SATBMarkQueueSet::flush_queue(SATBMarkQueue& queue) { diff --git a/src/hotspot/share/gc/shared/satbMarkQueue.hpp b/src/hotspot/share/gc/shared/satbMarkQueue.hpp index f1577c004de..36c287cd64d 100644 --- a/src/hotspot/share/gc/shared/satbMarkQueue.hpp +++ b/src/hotspot/share/gc/shared/satbMarkQueue.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2001, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2001, 2026, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -99,6 +99,10 @@ public: _index = index_to_byte_index(new_index); } + void set_empty() { + set_index(current_capacity()); + } + // Returns the capacity of the buffer, or 0 if the queue doesn't currently // have a buffer. size_t current_capacity() const; diff --git a/test/hotspot/jtreg/gc/g1/TestEagerReclaimHumongousRegions.java b/test/hotspot/jtreg/gc/g1/TestEagerReclaimHumongousRegions.java index 5637e578e8f..37902cad906 100644 --- a/test/hotspot/jtreg/gc/g1/TestEagerReclaimHumongousRegions.java +++ b/test/hotspot/jtreg/gc/g1/TestEagerReclaimHumongousRegions.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2014, 2026, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -109,7 +109,10 @@ public class TestEagerReclaimHumongousRegions { "-Xmx20M", "-Xms20m", "-XX:+UnlockDiagnosticVMOptions", + "-XX:+VerifyBeforeGC", "-XX:+VerifyAfterGC", + "-XX:+VerifyDuringGC", + "-XX:+G1VerifyBitmaps", "-Xbootclasspath/a:.", "-Xlog:gc=debug,gc+humongous=debug", "-XX:+UnlockDiagnosticVMOptions", diff --git a/test/hotspot/jtreg/gc/g1/TestVerificationInConcurrentCycle.java b/test/hotspot/jtreg/gc/g1/TestVerificationInConcurrentCycle.java index 5a69a6e5288..b5cf4c333a6 100644 --- a/test/hotspot/jtreg/gc/g1/TestVerificationInConcurrentCycle.java +++ b/test/hotspot/jtreg/gc/g1/TestVerificationInConcurrentCycle.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2022, 2026, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,6 +23,8 @@ package gc.g1; +import java.lang.ref.Reference; + /* * @test TestVerificationInConcurrentCycle * @requires vm.gc.G1 @@ -34,6 +36,7 @@ package gc.g1; * @run main/othervm * -Xbootclasspath/a:. * -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI + * -XX:G1HeapRegionSize=2m * -XX:+VerifyBeforeGC -XX:+VerifyDuringGC -XX:+VerifyAfterGC * -XX:+UseG1GC -XX:+G1VerifyHeapRegionCodeRoots * -XX:+G1VerifyBitmaps @@ -52,6 +55,7 @@ package gc.g1; * @run main/othervm * -Xbootclasspath/a:. * -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI + * -XX:G1HeapRegionSize=2m * -XX:+VerifyBeforeGC -XX:+VerifyDuringGC -XX:+VerifyAfterGC * -XX:+UseG1GC -XX:+G1VerifyHeapRegionCodeRoots * gc.g1.TestVerificationInConcurrentCycle @@ -64,27 +68,50 @@ public class TestVerificationInConcurrentCycle { private static final WhiteBox WB = WhiteBox.getWhiteBox(); + private static Object[] allocateHumongous() { + Object[] result = new Object[7]; + for (int i = 0; i < result.length; i++) { + result[i] = new byte[1024 * 1024]; // Is humongous. + } + return result; + } + + private static void dropHalf(Object[] array) { + for (int i = 0; i < array.length; i++) { + if (i % 2 == 0) { + array[i] = null; + } + } + } // All testN() assume initial state is idle, and restore that state. private static void testFullGCAt(String at) throws Exception { System.out.println("testSimpleCycle"); + + Object[] objects = allocateHumongous(); try { // Run one cycle. WB.concurrentGCRunTo(at); + dropHalf(objects); WB.fullGC(); } finally { WB.concurrentGCRunToIdle(); + Reference.reachabilityFence(objects); } } private static void testYoungGCAt(String at) throws Exception { System.out.println("testSimpleCycle"); + + Object[] objects = allocateHumongous(); try { // Run one cycle. WB.concurrentGCRunTo(at); + dropHalf(objects); WB.youngGC(); } finally { WB.concurrentGCRunToIdle(); + Reference.reachabilityFence(objects); } } diff --git a/test/hotspot/jtreg/gc/g1/pinnedobjs/TestDroppedRetainedTAMS.java b/test/hotspot/jtreg/gc/g1/pinnedobjs/TestDroppedRetainedTAMS.java index f650e53a25f..c4dd2900c86 100644 --- a/test/hotspot/jtreg/gc/g1/pinnedobjs/TestDroppedRetainedTAMS.java +++ b/test/hotspot/jtreg/gc/g1/pinnedobjs/TestDroppedRetainedTAMS.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024, 2026, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -30,9 +30,10 @@ * @build jdk.test.whitebox.WhiteBox * @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox * @run main/othervm -XX:+UseG1GC -XX:+UnlockDiagnosticVMOptions -XX:+UnlockExperimentalVMOptions - -XX:+WhiteBoxAPI -Xbootclasspath/a:. -Xmx32m -XX:G1NumCollectionsKeepPinned=1 - -XX:+VerifyBeforeGC -XX:+VerifyAfterGC -XX:G1MixedGCLiveThresholdPercent=100 - -XX:G1HeapWastePercent=0 -Xlog:gc,gc+ergo+cset=trace gc.g1.pinnedobjs.TestDroppedRetainedTAMS + * -XX:+WhiteBoxAPI -Xbootclasspath/a:. -Xmx32m -XX:G1NumCollectionsKeepPinned=1 + * -XX:+VerifyBeforeGC -XX:+VerifyAfterGC -XX:+VerifyDuringGC -XX:+G1VerifyBitmaps + * -XX:G1MixedGCLiveThresholdPercent=100 -XX:G1HeapWastePercent=0 + * -Xlog:gc,gc+ergo+cset=trace gc.g1.pinnedobjs.TestDroppedRetainedTAMS */ package gc.g1.pinnedobjs;