From 7082bb664a33cc2ca39d45e10d14da25e0bd1f84 Mon Sep 17 00:00:00 2001 From: rjgask Date: Fri, 17 Mar 2023 16:56:05 +0000 Subject: [PATCH 1/2] fix pause meausing --- runtime/mem/gc/g1/g1-gc.cpp | 23 +++++++++++------------ runtime/mem/gc/gc_stats.cpp | 4 ++-- runtime/mem/mem_stats.cpp | 6 ++++++ runtime/mem/mem_stats.h | 1 + 4 files changed, 20 insertions(+), 14 deletions(-) diff --git a/runtime/mem/gc/g1/g1-gc.cpp b/runtime/mem/gc/g1/g1-gc.cpp index 35c05f274..1b47b6ad0 100644 --- a/runtime/mem/gc/g1/g1-gc.cpp +++ b/runtime/mem/gc/g1/g1-gc.cpp @@ -617,6 +617,8 @@ bool G1GC::NeedFullGC(const panda::GCTask &task) template void G1GC::RunPhasesImpl(panda::GCTask &task) { + this->GetPandaVm()->GetMemStats()->RecordGCPauseStart(); + GCScopedPauseStats scoped_pause_stats(this->GetPandaVm()->GetGCStats()); interrupt_concurrent_flag_ = false; LOG_DEBUG_GC << "G1GC start, reason: " << task.reason; LOG_DEBUG_GC << "Footprint before GC: " << this->GetPandaVm()->GetMemStats()->GetFootprintHeap(); @@ -626,10 +628,8 @@ void G1GC::RunPhasesImpl(panda::GCTask &task) { ScopedTiming t("G1 GC", *this->GetTiming()); { - GCScopedPauseStats scoped_pause_stats(this->GetPandaVm()->GetGCStats()); this->mem_stats_.Reset(); if (NeedToRunGC(task)) { - this->GetPandaVm()->GetMemStats()->RecordGCPauseStart(); // Check there is no concurrent mark running by another thread. // Atomic with relaxed order reason: concurrent access with another thread which can running GC now ASSERT(pre_wrb_entrypoint_.load(std::memory_order_relaxed) == nullptr); @@ -664,16 +664,15 @@ void G1GC::RunPhasesImpl(panda::GCTask &task) : "not enough free regions to move"); } } - this->GetPandaVm()->GetMemStats()->RecordGCPauseEnd(); } + if (task.reason == GCTaskCause::MIXED) { + // There was forced a mixed GC. This GC type sets specific settings. + // So we need to restore them. + region_garbage_rate_threshold_ = this->GetSettings()->G1RegionGarbageRateThreshold(); + } + ScheduleMixedGCAndConcurrentMark(task); + RunConcurrentMarkIfNeeded(task); } - if (task.reason == GCTaskCause::MIXED) { - // There was forced a mixed GC. This GC type sets specific settings. - // So we need to restore them. - region_garbage_rate_threshold_ = this->GetSettings()->G1RegionGarbageRateThreshold(); - } - ScheduleMixedGCAndConcurrentMark(task); - RunConcurrentMarkIfNeeded(task); } // Update global and GC memstats based on generational memstats information // We will update tenured stats and record allocations, so set 'true' values @@ -681,6 +680,8 @@ void G1GC::RunPhasesImpl(panda::GCTask &task) LOG_DEBUG_GC << "Footprint after GC: " << this->GetPandaVm()->GetMemStats()->GetFootprintHeap(); this->SetFullGC(false); + + this->GetPandaVm()->GetMemStats()->RecordGCPauseEnd(); } template @@ -1280,7 +1281,6 @@ void G1GC::StartMarking(panda::GCTask &task) } // Concurrent/on-pause marking { - this->GetPandaVm()->GetMemStats()->RecordGCPauseEnd(); // NOLINTNEXTLINE(readability-braces-around-statements) if constexpr (IS_CONCURRENT) { const ReferenceCheckPredicateT &disable_ref_pred = []([[maybe_unused]] const ObjectHeader *obj) { @@ -1342,7 +1342,6 @@ void G1GC::StartMarking(panda::GCTask &task) concurrent_marking_stack_.Clear(); } ASSERT(concurrent_marking_stack_.Empty()); - this->GetPandaVm()->GetMemStats()->RecordGCPauseStart(); } ASSERT(concurrent_marking_stack_.Empty()); } diff --git a/runtime/mem/gc/gc_stats.cpp b/runtime/mem/gc/gc_stats.cpp index 6306ee9b1..c8aef1f55 100644 --- a/runtime/mem/gc/gc_stats.cpp +++ b/runtime/mem/gc/gc_stats.cpp @@ -233,13 +233,13 @@ GCScopedStats::~GCScopedStats() } GCScopedPauseStats::GCScopedPauseStats(GCStats *stats, GCInstanceStats *instance_stats) - : start_time_(time::GetCurrentTimeInNanos()), instance_stats_(instance_stats), stats_(stats) + : start_time_(stats->mem_stats_->GetTotalGCPauseInNanos()), instance_stats_(instance_stats), stats_(stats) { } GCScopedPauseStats::~GCScopedPauseStats() { - stats_->RecordPause(time::GetCurrentTimeInNanos() - start_time_, instance_stats_); + stats_->RecordPause(stats_->mem_stats_->GetTotalGCPauseInNanos() - start_time_, instance_stats_); } PandaString GCInstanceStats::GetDump(GCType gc_type) diff --git a/runtime/mem/mem_stats.cpp b/runtime/mem/mem_stats.cpp index d654620df..d0a33409b 100644 --- a/runtime/mem/mem_stats.cpp +++ b/runtime/mem/mem_stats.cpp @@ -233,6 +233,12 @@ uint64_t MemStats::GetTotalGCPause() const return std::chrono::duration_cast(sum_pause_).count(); } +template +uint64_t MemStats::GetTotalGCPauseInNanos() const +{ + return std::chrono::duration_cast(sum_pause_).count(); +} + template class MemStats; template class MemStats; } // namespace panda::mem diff --git a/runtime/mem/mem_stats.h b/runtime/mem/mem_stats.h index 4fc37115d..7d52fdb17 100644 --- a/runtime/mem/mem_stats.h +++ b/runtime/mem/mem_stats.h @@ -123,6 +123,7 @@ public: uint64_t GetMaxGCPause() const; uint64_t GetAverageGCPause() const; uint64_t GetTotalGCPause() const; + uint64_t GetTotalGCPauseInNanos() const; protected: using Clock = std::chrono::high_resolution_clock; -- Gitee From 2457990cc15a2ec768a1979cce5582712a65b9f0 Mon Sep 17 00:00:00 2001 From: rjgask Date: Fri, 17 Mar 2023 20:40:46 +0000 Subject: [PATCH 2/2] defer --- runtime/mem/gc/g1/g1-gc.cpp | 4 ---- runtime/mem/gc/g1/update_remset_thread.cpp | 17 +++++++++++++++++ runtime/mem/gc/g1/update_remset_thread.h | 4 ++++ 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/runtime/mem/gc/g1/g1-gc.cpp b/runtime/mem/gc/g1/g1-gc.cpp index 1b47b6ad0..8cea94628 100644 --- a/runtime/mem/gc/g1/g1-gc.cpp +++ b/runtime/mem/gc/g1/g1-gc.cpp @@ -1389,10 +1389,6 @@ void G1GC::Remark(panda::GCTask const &task) if (use_gc_workers) { this->GetWorkersPool()->WaitUntilTasksEnd(); } - { - ScopedTiming remset_thread_timing("RemsetThread WaitUntilTasksEnd", *this->GetTiming()); - WaitForUpdateRemsetThread(); - } // ConcurrentMark doesn't visit young objects - so we can't clear references which are in young-space because we // don't know which objects are marked. We will process them on young/mixed GC separately later, here we process diff --git a/runtime/mem/gc/g1/update_remset_thread.cpp b/runtime/mem/gc/g1/update_remset_thread.cpp index 4f236ae92..e8343e83a 100644 --- a/runtime/mem/gc/g1/update_remset_thread.cpp +++ b/runtime/mem/gc/g1/update_remset_thread.cpp @@ -157,10 +157,27 @@ void UpdateRemsetThread::ThreadLoop() continue; } if (invalidate_regions_ != nullptr) { + FillFromDefered(&cards_); + FillFromQueue(&cards_); + FillFromThreads(&cards_); + + PandaSet region_set; for (const auto ®ion : *invalidate_regions_) { // don't need lock because only update_remset_thread changes remsets RemSet<>::template InvalidateRegion(region); + region_set.insert(region); + } + + for (auto it = cards_.begin(); it != cards_.end();) { + auto start_address = ToVoidPtr(card_table_->GetCardStartAddress(*it)); + auto region = AddrToRegion(start_address); + if (region_set.count(region) == 0) { + ++it; + continue; + } + it = cards_.erase(it); } + invalidate_regions_ = nullptr; thread_cond_var_.Signal(); Sleep(); diff --git a/runtime/mem/gc/g1/update_remset_thread.h b/runtime/mem/gc/g1/update_remset_thread.h index dfb150d4c..568102656 100644 --- a/runtime/mem/gc/g1/update_remset_thread.h +++ b/runtime/mem/gc/g1/update_remset_thread.h @@ -121,6 +121,8 @@ public: void InvalidateRegions(PandaVector *regions) { + // Atomic with relaxed order reason: memory order is not required + defer_cards_.store(true, std::memory_order_relaxed); need_invalidate_region_ = true; { os::memory::LockHolder holder(loop_lock_); @@ -132,6 +134,8 @@ public: while (invalidate_regions_ != nullptr) { Sleep(); } + // Atomic with relaxed order reason: memory order is not required + defer_cards_.store(false, std::memory_order_relaxed); } need_invalidate_region_ = false; thread_cond_var_.Signal(); -- Gitee