Skip to content

Commit

Permalink
[vm] Fix HeapProfileSampler::SampleOldSpaceAllocation
Browse files Browse the repository at this point in the history
When Isolate is just starting up and no TLABs are yet present
we were setting interval_to_next_sample_ to -1 which caused
every old space allocation to be sampled.

This CL properly initializes the interval to sampling_interval_
instead.

TEST=vm/cc/DartAPI_HeapSampling_CorrectSamplingIntervalForOldSpaceAllocations

Change-Id: Ieb29234379d3afa4716ebdf3591f9fc0cbcaef71
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/311844
Commit-Queue: Slava Egorov <[email protected]>
Commit-Queue: Martin Kustermann <[email protected]>
Auto-Submit: Slava Egorov <[email protected]>
Reviewed-by: Martin Kustermann <[email protected]>
  • Loading branch information
mraleph authored and Commit Queue committed Jun 28, 2023
1 parent 8f7cf77 commit 86c3116
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 0 deletions.
23 changes: 23 additions & 0 deletions runtime/vm/dart_api_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10868,6 +10868,29 @@ TEST_CASE(DartAPI_HeapSampling_NonTrivialSamplingPeriod) {
Dart_EnterIsolate(isolate);
Dart_DisableHeapSampling();
}

// Check that we set correct sampling interval for old space allocations
// during initial isolate startup (before the first TLAB is created).
// This is a regression test for a bug that was causing each old space
// allocation to be sampled.
VM_UNIT_TEST_CASE(
DartAPI_HeapSampling_CorrectSamplingIntervalForOldSpaceAllocations) {
Dart_RegisterHeapSamplingCallback(
[](Dart_Isolate isolate, Dart_IsolateGroup isolate_group,
const char* cls_name, intptr_t allocation_size) -> void* {
// Should not be called because sampling interval is very large.
UNREACHABLE();
return nullptr;
},
[](void* data) {
// Nothing to do.
});
Dart_SetHeapSamplingPeriod(1 * GB);
Dart_EnableHeapSampling();
TestCase::CreateTestIsolate();
Dart_DisableHeapSampling();
Dart_ShutdownIsolate();
}
#endif // !defined(PRODUCT) || defined(FORCE_INCLUDE_SAMPLING_HEAP_PROFILER)

#if defined(DART_ENABLE_HEAP_SNAPSHOT_WRITER)
Expand Down
6 changes: 6 additions & 0 deletions runtime/vm/heap/sampler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,12 @@ void HeapProfileSampler::SampleOldSpaceAllocation(intptr_t allocation_size) {
interval_to_next_sample_ = tlab_interval;
}

// If we don't have a TLAB yet simply initialize interval_to_next_sample_
// from the sampling interval.
if (interval_to_next_sample_ == kUninitialized) {
interval_to_next_sample_ = sampling_interval_;
}

// Check the allocation is large enough to trigger a sample. If not, tighten
// the interval.
if (allocation_size < interval_to_next_sample_) {
Expand Down

0 comments on commit 86c3116

Please sign in to comment.