Skip to content

Commit adda4c5

Browse files
omerktzCommit Bot
authored andcommitted
cppgc: Add UMA support
This CL introduces cppgc::HistogramRecorder api which is similar to the v8::metrics::Recorder api and is used by cppgc to report histogram samples to embedders. Embedders should implement the api if they want to collect histograms and provide an instance of it on heap creation. CppHeap uses an adaptor class that implements the HistogramRecorder api and is used to forward the relevant info to the relevant v8::metrics::Recorder. The api used 3 data structures: 2 for incremental steps that need to be reported as they come (marking and sweeping) and 1 for the end of a GC cycle that aggregates statistics over the entire cycle. The data structure only provide the "raw" samples (e.g. atomic mark time, incremental mark time, etc...). The embedder is expected to compute aggregate histogram on its own (e.g. overall marking time). Bug: chromium:1056170 Change-Id: If63ef50a29a21594f654edb83084598980d221ce Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2642258 Commit-Queue: Omer Katz <[email protected]> Reviewed-by: Michael Lippautz <[email protected]> Reviewed-by: Ulan Degenbaev <[email protected]> Cr-Commit-Position: refs/heads/master@{#72256}
1 parent 987f0b7 commit adda4c5

17 files changed

+551
-50
lines changed

BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4646,6 +4646,7 @@ v8_source_set("cppgc_base") {
46464646
"src/heap/cppgc/marking-visitor.h",
46474647
"src/heap/cppgc/marking-worklists.cc",
46484648
"src/heap/cppgc/marking-worklists.h",
4649+
"src/heap/cppgc/metric-recorder.h",
46494650
"src/heap/cppgc/name-trait.cc",
46504651
"src/heap/cppgc/object-allocator.cc",
46514652
"src/heap/cppgc/object-allocator.h",

src/heap/cppgc-js/cpp-heap.cc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,11 +169,13 @@ void UnifiedHeapMarker::AddObject(void* object) {
169169

170170
CppHeap::CppHeap(
171171
v8::Isolate* isolate,
172-
const std::vector<std::unique_ptr<cppgc::CustomSpaceBase>>& custom_spaces)
172+
const std::vector<std::unique_ptr<cppgc::CustomSpaceBase>>& custom_spaces,
173+
std::unique_ptr<cppgc::internal::MetricRecorder> metric_recorder)
173174
: cppgc::internal::HeapBase(std::make_shared<CppgcPlatformAdapter>(isolate),
174175
custom_spaces,
175176
cppgc::internal::HeapBase::StackSupport::
176-
kSupportsConservativeStackScan),
177+
kSupportsConservativeStackScan,
178+
std::move(metric_recorder)),
177179
isolate_(*reinterpret_cast<Isolate*>(isolate)) {
178180
if (isolate_.heap_profiler()) {
179181
isolate_.heap_profiler()->AddBuildEmbedderGraphCallback(

src/heap/cppgc-js/cpp-heap.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,11 @@ class V8_EXPORT_PRIVATE CppHeap final : public cppgc::internal::HeapBase,
2828
return static_cast<const CppHeap*>(heap);
2929
}
3030

31-
CppHeap(v8::Isolate* isolate,
32-
const std::vector<std::unique_ptr<cppgc::CustomSpaceBase>>&
33-
custom_spaces);
31+
CppHeap(
32+
v8::Isolate* isolate,
33+
const std::vector<std::unique_ptr<cppgc::CustomSpaceBase>>& custom_spaces,
34+
std::unique_ptr<cppgc::internal::MetricRecorder> metric_recorder =
35+
nullptr);
3436
~CppHeap() final;
3537

3638
CppHeap(const CppHeap&) = delete;
@@ -56,8 +58,6 @@ class V8_EXPORT_PRIVATE CppHeap final : public cppgc::internal::HeapBase,
5658
// finalization is not needed) thus this method is left empty.
5759
}
5860

59-
void PostGarbageCollection() final {}
60-
6161
Isolate& isolate_;
6262
bool marking_done_ = false;
6363
bool is_in_final_pause_ = false;

src/heap/cppgc/heap-base.cc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ class ObjectSizeCounter : private HeapVisitor<ObjectSizeCounter> {
5656
HeapBase::HeapBase(
5757
std::shared_ptr<cppgc::Platform> platform,
5858
const std::vector<std::unique_ptr<CustomSpaceBase>>& custom_spaces,
59-
StackSupport stack_support)
59+
StackSupport stack_support,
60+
std::unique_ptr<MetricRecorder> histogram_recorder)
6061
: raw_heap_(this, custom_spaces),
6162
platform_(std::move(platform)),
6263
#if defined(CPPGC_CAGED_HEAP)
@@ -66,7 +67,8 @@ HeapBase::HeapBase(
6667
page_backend_(
6768
std::make_unique<PageBackend>(platform_->GetPageAllocator())),
6869
#endif
69-
stats_collector_(std::make_unique<StatsCollector>()),
70+
stats_collector_(
71+
std::make_unique<StatsCollector>(std::move(histogram_recorder))),
7072
stack_(std::make_unique<heap::base::Stack>(
7173
v8::base::Stack::GetStackStart())),
7274
prefinalizer_handler_(std::make_unique<PreFinalizerHandler>(*this)),

src/heap/cppgc/heap-base.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "src/base/macros.h"
1515
#include "src/heap/cppgc/compactor.h"
1616
#include "src/heap/cppgc/marker.h"
17+
#include "src/heap/cppgc/metric-recorder.h"
1718
#include "src/heap/cppgc/object-allocator.h"
1819
#include "src/heap/cppgc/raw-heap.h"
1920
#include "src/heap/cppgc/sweeper.h"
@@ -79,7 +80,8 @@ class V8_EXPORT_PRIVATE HeapBase : public cppgc::HeapHandle {
7980

8081
HeapBase(std::shared_ptr<cppgc::Platform> platform,
8182
const std::vector<std::unique_ptr<CustomSpaceBase>>& custom_spaces,
82-
StackSupport stack_support);
83+
StackSupport stack_support,
84+
std::unique_ptr<MetricRecorder> histogram_recorder);
8385
virtual ~HeapBase();
8486

8587
HeapBase(const HeapBase&) = delete;
@@ -153,9 +155,6 @@ class V8_EXPORT_PRIVATE HeapBase : public cppgc::HeapHandle {
153155

154156
void AdvanceIncrementalGarbageCollectionOnAllocationIfNeeded();
155157

156-
// Notifies the heap that a GC is done.
157-
virtual void PostGarbageCollection() = 0;
158-
159158
// Termination drops all roots (clears them out) and runs garbage collections
160159
// in a bounded fixed point loop until no new objects are created in
161160
// destructors. Exceeding the loop bound results in a crash.

src/heap/cppgc/heap.cc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ void CheckConfig(Heap::Config config, Heap::MarkingType marking_support,
8686

8787
Heap::Heap(std::shared_ptr<cppgc::Platform> platform,
8888
cppgc::Heap::HeapOptions options)
89-
: HeapBase(platform, options.custom_spaces, options.stack_support),
89+
: HeapBase(platform, options.custom_spaces, options.stack_support,
90+
nullptr /* metric_recorder */),
9091
gc_invoker_(this, platform_.get(), options.stack_support),
9192
growing_(&gc_invoker_, stats_collector_.get(),
9293
options.resource_constraints, options.marking_support,
@@ -196,8 +197,6 @@ void Heap::FinalizeGarbageCollection(Config::StackState stack_state) {
196197
sweeper_.NotifyDoneIfNeeded();
197198
}
198199

199-
void Heap::PostGarbageCollection() {}
200-
201200
void Heap::DisableHeapGrowingForTesting() { growing_.DisableForTesting(); }
202201

203202
void Heap::FinalizeIncrementalGarbageCollectionIfNeeded(

src/heap/cppgc/heap.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,6 @@ class V8_EXPORT_PRIVATE Heap final : public HeapBase,
4646

4747
void FinalizeIncrementalGarbageCollectionIfNeeded(Config::StackState) final;
4848

49-
void PostGarbageCollection() final;
50-
5149
Config config_;
5250
GCInvoker gc_invoker_;
5351
HeapGrowing growing_;

src/heap/cppgc/marker.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,8 +287,8 @@ void MarkerBase::FinishMarking(MarkingConfig::StackState stack_state) {
287287
void MarkerBase::ProcessWeakness() {
288288
DCHECK_EQ(MarkingConfig::MarkingType::kAtomic, config_.marking_type);
289289

290-
StatsCollector::DisabledScope stats_scope(
291-
heap(), StatsCollector::kWeakInvokeCallbacks);
290+
StatsCollector::DisabledScope stats_scope(heap(),
291+
StatsCollector::kAtomicWeak);
292292

293293
heap().GetWeakPersistentRegion().Trace(&visitor());
294294
// Processing cross-thread handles requires taking the process lock.

src/heap/cppgc/metric-recorder.h

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// Copyright 2021 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#ifndef V8_HEAP_CPPGC_METRIC_RECORDER_H_
6+
#define V8_HEAP_CPPGC_METRIC_RECORDER_H_
7+
8+
#include <cstdint>
9+
10+
namespace cppgc {
11+
namespace internal {
12+
13+
class StatsCollector;
14+
15+
/**
16+
* Base class used for reporting GC statistics histograms. Embedders interested
17+
* in collecting histgorams should implement the virtual AddMainThreadEvent
18+
* methods below and pass an instance of the implementation during Heap
19+
* creation.
20+
*/
21+
class MetricRecorder {
22+
public:
23+
struct CppGCCycleEndMetricSamples {
24+
int64_t atomic_mark_ms;
25+
int64_t atomic_weak_ms;
26+
int64_t atomic_compact_ms;
27+
int64_t atomic_sweep_ms;
28+
int64_t incremental_mark_ms;
29+
int64_t incremental_sweep_ms;
30+
int64_t concurrent_mark_ms;
31+
int64_t concurrent_sweep_ms;
32+
};
33+
34+
struct CppGCIncrementalMarkMetricSample {
35+
int64_t duration_ms;
36+
};
37+
38+
struct CppGCIncrementalSweepMetricSample {
39+
int64_t duration_ms;
40+
};
41+
42+
virtual ~MetricRecorder() = default;
43+
44+
virtual void AddMainThreadEvent(const CppGCCycleEndMetricSamples& event) {}
45+
virtual void AddMainThreadEvent(
46+
const CppGCIncrementalMarkMetricSample& event) {}
47+
virtual void AddMainThreadEvent(
48+
const CppGCIncrementalSweepMetricSample& event) {}
49+
};
50+
51+
} // namespace internal
52+
} // namespace cppgc
53+
54+
#endif // V8_HEAP_CPPGC_METRIC_RECORDER_H_

src/heap/cppgc/stats-collector.cc

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,18 @@
88
#include <cmath>
99

1010
#include "src/base/logging.h"
11+
#include "src/heap/cppgc/metric-recorder.h"
1112

1213
namespace cppgc {
1314
namespace internal {
1415

1516
// static
1617
constexpr size_t StatsCollector::kAllocationThresholdBytes;
1718

19+
StatsCollector::StatsCollector(
20+
std::unique_ptr<MetricRecorder> histogram_recorder)
21+
: metric_recorder_(std::move(histogram_recorder)) {}
22+
1823
void StatsCollector::RegisterObserver(AllocationObserver* observer) {
1924
DCHECK_EQ(allocation_observers_.end(),
2025
std::find(allocation_observers_.begin(),
@@ -114,6 +119,18 @@ void StatsCollector::NotifySweepingCompleted() {
114119
gc_state_ = GarbageCollectionState::kNotRunning;
115120
previous_ = std::move(current_);
116121
current_ = Event();
122+
if (metric_recorder_) {
123+
MetricRecorder::CppGCCycleEndMetricSamples event{
124+
previous_.scope_data[kAtomicMark].InMilliseconds(),
125+
previous_.scope_data[kAtomicWeak].InMilliseconds(),
126+
previous_.scope_data[kAtomicCompact].InMilliseconds(),
127+
previous_.scope_data[kAtomicSweep].InMilliseconds(),
128+
previous_.scope_data[kIncrementalMark].InMilliseconds(),
129+
previous_.scope_data[kIncrementalSweep].InMilliseconds(),
130+
previous_.concurrent_scope_data[kConcurrentMark],
131+
previous_.concurrent_scope_data[kConcurrentSweep]};
132+
metric_recorder_->AddMainThreadEvent(event);
133+
}
117134
}
118135

119136
size_t StatsCollector::allocated_object_size() const {
@@ -129,5 +146,25 @@ size_t StatsCollector::allocated_object_size() const {
129146
allocated_bytes_since_end_of_marking_);
130147
}
131148

149+
void StatsCollector::RecordHistogramSample(ScopeId scope_id_,
150+
v8::base::TimeDelta time) {
151+
switch (scope_id_) {
152+
case kIncrementalMark: {
153+
MetricRecorder::CppGCIncrementalMarkMetricSample event{
154+
time.InMilliseconds()};
155+
metric_recorder_->AddMainThreadEvent(event);
156+
break;
157+
}
158+
case kIncrementalSweep: {
159+
MetricRecorder::CppGCIncrementalSweepMetricSample event{
160+
time.InMilliseconds()};
161+
metric_recorder_->AddMainThreadEvent(event);
162+
break;
163+
}
164+
default:
165+
break;
166+
}
167+
}
168+
132169
} // namespace internal
133170
} // namespace cppgc

0 commit comments

Comments
 (0)