diff --git a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp index d18f61ff507..a9cce3e9881 100644 --- a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp +++ b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp @@ -1330,7 +1330,6 @@ G1CollectedHeap::G1CollectedHeap() : _card_set_freelist_pool(G1CardSetConfiguration::num_mem_object_types()), _young_regions_cset_group(card_set_config(), &_card_set_freelist_pool, G1CSetCandidateGroup::YoungRegionId), _cm(nullptr), - _cm_thread(nullptr), _cr(nullptr), _task_queues(nullptr), _partial_array_state_manager(nullptr), @@ -1574,7 +1573,6 @@ jint G1CollectedHeap::initialize() { // Create the G1ConcurrentMark data structure and thread. // (Must do this late, so that "max_[reserved_]regions" is defined.) _cm = new G1ConcurrentMark(this, bitmap_storage); - _cm_thread = _cm->cm_thread(); // Now expand into the initial heap size. if (!expand(init_byte_size, _workers)) { @@ -1651,7 +1649,9 @@ void G1CollectedHeap::stop() { // that are destroyed during shutdown. _cr->stop(); _service_thread->stop(); - _cm_thread->stop(); + if (_cm->is_fully_initialized()) { + _cm->cm_thread()->stop(); + } } void G1CollectedHeap::safepoint_synchronize_begin() { @@ -1848,7 +1848,7 @@ void G1CollectedHeap::increment_old_marking_cycles_completed(bool concurrent, // is set) so that if a waiter requests another System.gc() it doesn't // incorrectly see that a marking cycle is still in progress. if (concurrent) { - _cm_thread->set_idle(); + _cm->cm_thread()->set_idle(); } // Notify threads waiting in System.gc() (with ExplicitGCInvokesConcurrent) @@ -2426,7 +2426,9 @@ void G1CollectedHeap::print_gc_on(outputStream* st) const { void G1CollectedHeap::gc_threads_do(ThreadClosure* tc) const { workers()->threads_do(tc); - tc->do_thread(_cm_thread); + if (_cm->is_fully_initialized()) { + tc->do_thread(_cm->cm_thread()); + } _cm->threads_do(tc); _cr->threads_do(tc); tc->do_thread(_service_thread); @@ -2547,15 +2549,15 @@ HeapWord* G1CollectedHeap::do_collection_pause(size_t word_size, } void G1CollectedHeap::start_concurrent_cycle(bool concurrent_operation_is_full_mark) { - assert(!_cm_thread->in_progress(), "Can not start concurrent operation while in progress"); - + assert(!_cm->in_progress(), "Can not start concurrent operation while in progress"); + assert(_cm->is_fully_initialized(), "sanity"); MutexLocker x(G1CGC_lock, Mutex::_no_safepoint_check_flag); if (concurrent_operation_is_full_mark) { _cm->post_concurrent_mark_start(); - _cm_thread->start_full_mark(); + _cm->cm_thread()->start_full_mark(); } else { _cm->post_concurrent_undo_start(); - _cm_thread->start_undo_mark(); + _cm->cm_thread()->start_undo_mark(); } G1CGC_lock->notify(); } diff --git a/src/hotspot/share/gc/g1/g1CollectedHeap.hpp b/src/hotspot/share/gc/g1/g1CollectedHeap.hpp index 5dccf41e909..93810bbaa60 100644 --- a/src/hotspot/share/gc/g1/g1CollectedHeap.hpp +++ b/src/hotspot/share/gc/g1/g1CollectedHeap.hpp @@ -824,7 +824,6 @@ public: // The concurrent marker (and the thread it runs in.) G1ConcurrentMark* _cm; - G1ConcurrentMarkThread* _cm_thread; // The concurrent refiner. G1ConcurrentRefine* _cr; diff --git a/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp b/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp index d37fe9ea7ba..18e8b37c03e 100644 --- a/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp +++ b/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp @@ -470,7 +470,7 @@ bool G1CMRootMemRegions::wait_until_scan_finished() { G1ConcurrentMark::G1ConcurrentMark(G1CollectedHeap* g1h, G1RegionToSpaceMapper* bitmap_storage) : - // _cm_thread set inside the constructor + _cm_thread(nullptr), _g1h(g1h), _mark_bitmap(), @@ -481,13 +481,12 @@ G1ConcurrentMark::G1ConcurrentMark(G1CollectedHeap* g1h, _global_mark_stack(), - // _finger set in set_non_marking_state + _finger(nullptr), // _finger set in set_non_marking_state _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 set in set_non_marking_state() - // _tasks set inside the constructor - + _num_active_tasks(0), // _num_active_tasks set in set_non_marking_state() + _tasks(nullptr), // _tasks set inside late_init() _task_queues(new G1CMTaskQueueSet(_max_num_tasks)), _terminator(_max_num_tasks, _task_queues), @@ -521,6 +520,32 @@ G1ConcurrentMark::G1ConcurrentMark(G1CollectedHeap* g1h, assert(G1CGC_lock != nullptr, "CGC_lock must be initialized"); _mark_bitmap.initialize(g1h->reserved(), bitmap_storage); +} + +void G1ConcurrentMark::reset() { + _has_aborted = false; + + reset_marking_for_restart(); + + // Reset all tasks, since different phases will use different number of active + // threads. So, it's easiest to have all of them ready. + for (uint i = 0; i < _max_num_tasks; ++i) { + _tasks[i]->reset(mark_bitmap()); + } + + uint max_num_regions = _g1h->max_num_regions(); + for (uint i = 0; i < max_num_regions; i++) { + _top_at_rebuild_starts[i] = nullptr; + _region_mark_stats[i].clear(); + } + + _root_regions.reset(); +} + +void G1ConcurrentMark::fully_initialize() { + if (is_fully_initialized()) { + return; + } // Create & start ConcurrentMark thread. _cm_thread = new G1ConcurrentMarkThread(this); @@ -556,24 +581,8 @@ G1ConcurrentMark::G1ConcurrentMark(G1CollectedHeap* g1h, reset_at_marking_complete(); } -void G1ConcurrentMark::reset() { - _has_aborted = false; - - reset_marking_for_restart(); - - // Reset all tasks, since different phases will use different number of active - // threads. So, it's easiest to have all of them ready. - for (uint i = 0; i < _max_num_tasks; ++i) { - _tasks[i]->reset(mark_bitmap()); - } - - uint max_num_regions = _g1h->max_num_regions(); - for (uint i = 0; i < max_num_regions; i++) { - _top_at_rebuild_starts[i] = nullptr; - _region_mark_stats[i].clear(); - } - - _root_regions.reset(); +bool G1ConcurrentMark::in_progress() const { + return is_fully_initialized() ? _cm_thread->in_progress() : false; } void G1ConcurrentMark::clear_statistics(G1HeapRegion* r) { @@ -738,7 +747,7 @@ private: // as asserts here to minimize their overhead on the product. However, we // will have them as guarantees at the beginning / end of the bitmap // clearing to get some checking in the product. - assert(!suspendible() || _cm->cm_thread()->in_progress(), "invariant"); + assert(!suspendible() || _cm->in_progress(), "invariant"); assert(!suspendible() || !G1CollectedHeap::heap()->collector_state()->mark_or_rebuild_in_progress(), "invariant"); // Abort iteration if necessary. @@ -794,7 +803,8 @@ void G1ConcurrentMark::clear_bitmap(WorkerThreads* workers, bool may_yield) { void G1ConcurrentMark::cleanup_for_next_mark() { // Make sure that the concurrent mark thread looks to still be in // the current cycle. - guarantee(cm_thread()->in_progress(), "invariant"); + guarantee(is_fully_initialized(), "should be initializd"); + guarantee(in_progress(), "invariant"); // We are finishing up the current cycle by clearing the next // marking bitmap and getting it ready for the next cycle. During @@ -805,7 +815,8 @@ void G1ConcurrentMark::cleanup_for_next_mark() { clear_bitmap(_concurrent_workers, true); // Repeat the asserts from above. - guarantee(cm_thread()->in_progress(), "invariant"); + guarantee(is_fully_initialized(), "should be initializd"); + guarantee(in_progress(), "invariant"); guarantee(!_g1h->collector_state()->mark_or_rebuild_in_progress(), "invariant"); } @@ -1883,7 +1894,7 @@ bool G1ConcurrentMark::concurrent_cycle_abort() { // nothing, but this situation should be extremely rare (a full gc after shutdown // has been signalled is already rare), and this work should be negligible compared // to actual full gc work. - if (!cm_thread()->in_progress() && !_g1h->is_shutting_down()) { + if (!is_fully_initialized() || (!cm_thread()->in_progress() && !_g1h->is_shutting_down())) { return false; } @@ -1945,6 +1956,10 @@ void G1ConcurrentMark::print_summary_info() { } log.trace(" Concurrent marking:"); + if (!is_fully_initialized()) { + log.trace(" has not been initialized yet"); + return; + } print_ms_time_info(" ", "remarks", _remark_times); { print_ms_time_info(" ", "final marks", _remark_mark_times); @@ -1961,7 +1976,9 @@ void G1ConcurrentMark::print_summary_info() { } void G1ConcurrentMark::threads_do(ThreadClosure* tc) const { - _concurrent_workers->threads_do(tc); + if (is_fully_initialized()) { // they are initialized late + _concurrent_workers->threads_do(tc); + } } void G1ConcurrentMark::print_on(outputStream* st) const { diff --git a/src/hotspot/share/gc/g1/g1ConcurrentMark.hpp b/src/hotspot/share/gc/g1/g1ConcurrentMark.hpp index 752082ce629..e9730bc1585 100644 --- a/src/hotspot/share/gc/g1/g1ConcurrentMark.hpp +++ b/src/hotspot/share/gc/g1/g1ConcurrentMark.hpp @@ -582,6 +582,10 @@ public: uint worker_id_offset() const { return _worker_id_offset; } + void fully_initialize(); + bool is_fully_initialized() const { return _cm_thread != nullptr; } + bool in_progress() const; + // Clear statistics gathered during the concurrent cycle for the given region after // it has been reclaimed. void clear_statistics(G1HeapRegion* r); diff --git a/src/hotspot/share/gc/g1/g1PeriodicGCTask.cpp b/src/hotspot/share/gc/g1/g1PeriodicGCTask.cpp index 50002ac2bfe..92172447cb6 100644 --- a/src/hotspot/share/gc/g1/g1PeriodicGCTask.cpp +++ b/src/hotspot/share/gc/g1/g1PeriodicGCTask.cpp @@ -38,8 +38,14 @@ bool G1PeriodicGCTask::should_start_periodic_gc(G1CollectedHeap* g1h, // Ensure no GC safepoints while we're doing the checks, to avoid data races. SuspendibleThreadSetJoiner sts; + // We should not start a concurrent gc if concurrent marking has not been initialized yet + if (!g1h->concurrent_mark()->is_fully_initialized()) { + log_debug(gc, periodic)("Concurrent marking has not been initialized. Skipping."); + return false; + } + // If we are currently in a concurrent mark we are going to uncommit memory soon. - if (g1h->concurrent_mark()->cm_thread()->in_progress()) { + if (g1h->concurrent_mark()->in_progress()) { log_debug(gc, periodic)("Concurrent cycle in progress. Skipping."); return false; } diff --git a/src/hotspot/share/gc/g1/g1Policy.cpp b/src/hotspot/share/gc/g1/g1Policy.cpp index 19573e11cd7..858f4d1df66 100644 --- a/src/hotspot/share/gc/g1/g1Policy.cpp +++ b/src/hotspot/share/gc/g1/g1Policy.cpp @@ -740,7 +740,10 @@ double G1Policy::constant_other_time_ms(double pause_time_ms) const { } bool G1Policy::about_to_start_mixed_phase() const { - return _g1h->concurrent_mark()->cm_thread()->in_progress() || collector_state()->in_young_gc_before_mixed(); + if (!_g1h->concurrent_mark()->is_fully_initialized()) { + return false; + } + return _g1h->concurrent_mark()->in_progress() || collector_state()->in_young_gc_before_mixed(); } bool G1Policy::need_to_start_conc_mark(const char* source, size_t allocation_word_size) { @@ -1239,7 +1242,7 @@ bool G1Policy::force_concurrent_start_if_outside_cycle(GCCause::Cause gc_cause) // We actually check whether we are marking here and not if we are in a // reclamation phase. This means that we will schedule a concurrent mark // even while we are still in the process of reclaiming memory. - bool during_cycle = _g1h->concurrent_mark()->cm_thread()->in_progress(); + bool during_cycle = _g1h->concurrent_mark()->in_progress(); if (!during_cycle) { log_debug(gc, ergo)("Request concurrent cycle initiation (requested by GC cause). " "GC cause: %s", diff --git a/src/hotspot/share/gc/g1/g1RegionMarkStatsCache.cpp b/src/hotspot/share/gc/g1/g1RegionMarkStatsCache.cpp index d9b7ec294bd..c5f55e1d20c 100644 --- a/src/hotspot/share/gc/g1/g1RegionMarkStatsCache.cpp +++ b/src/hotspot/share/gc/g1/g1RegionMarkStatsCache.cpp @@ -29,12 +29,12 @@ G1RegionMarkStatsCache::G1RegionMarkStatsCache(G1RegionMarkStats* target, uint num_cache_entries) : _target(target), + _cache(NEW_C_HEAP_ARRAY(G1RegionMarkStatsCacheEntry, num_cache_entries, mtGC)), _num_cache_entries(num_cache_entries), _num_cache_entries_mask(_num_cache_entries - 1) { guarantee(is_power_of_2(num_cache_entries), "Number of cache entries must be power of two, but is %u", num_cache_entries); - _cache = NEW_C_HEAP_ARRAY(G1RegionMarkStatsCacheEntry, _num_cache_entries, mtGC); } G1RegionMarkStatsCache::~G1RegionMarkStatsCache() { diff --git a/src/hotspot/share/gc/g1/g1VMOperations.cpp b/src/hotspot/share/gc/g1/g1VMOperations.cpp index 1c024f2943b..56ab3a4b0fe 100644 --- a/src/hotspot/share/gc/g1/g1VMOperations.cpp +++ b/src/hotspot/share/gc/g1/g1VMOperations.cpp @@ -85,7 +85,7 @@ void VM_G1TryInitiateConcMark::doit() { GCCauseSetter x(g1h, _gc_cause); _mark_in_progress = g1h->collector_state()->mark_in_progress(); - _cycle_already_in_progress = g1h->concurrent_mark()->cm_thread()->in_progress(); + _cycle_already_in_progress = g1h->concurrent_mark()->in_progress(); if (!g1h->policy()->force_concurrent_start_if_outside_cycle(_gc_cause)) { // Failure to force the next GC pause to be a concurrent start indicates diff --git a/src/hotspot/share/gc/g1/g1YoungCollector.cpp b/src/hotspot/share/gc/g1/g1YoungCollector.cpp index fa5c617f1db..f5bca9a68b0 100644 --- a/src/hotspot/share/gc/g1/g1YoungCollector.cpp +++ b/src/hotspot/share/gc/g1/g1YoungCollector.cpp @@ -1124,6 +1124,7 @@ G1YoungCollector::G1YoungCollector(GCCause::Cause gc_cause, } void G1YoungCollector::collect() { + _g1h->_cm->fully_initialize(); // Do timing/tracing/statistics/pre- and post-logging/verification work not // directly related to the collection. They should not be accounted for in // collection work timing. diff --git a/src/hotspot/share/prims/whitebox.cpp b/src/hotspot/share/prims/whitebox.cpp index 5514f7d3260..3d3d018181a 100644 --- a/src/hotspot/share/prims/whitebox.cpp +++ b/src/hotspot/share/prims/whitebox.cpp @@ -567,7 +567,7 @@ WB_END WB_ENTRY(jboolean, WB_G1InConcurrentMark(JNIEnv* env, jobject o)) if (UseG1GC) { G1CollectedHeap* g1h = G1CollectedHeap::heap(); - return g1h->concurrent_mark()->cm_thread()->in_progress(); + return g1h->concurrent_mark()->in_progress(); } THROW_MSG_0(vmSymbols::java_lang_UnsupportedOperationException(), "WB_G1InConcurrentMark: G1 GC is not enabled"); WB_END