Skip to content

Commit

Permalink
[ABI-stability] Revert "[api] Add v8::metrics::LongTaskStats for the …
Browse files Browse the repository at this point in the history
…LongTasks UKM"

This reverts commit 521ae93.
  • Loading branch information
targos committed Jul 22, 2021
1 parent 0503055 commit e408cc2
Show file tree
Hide file tree
Showing 8 changed files with 8 additions and 159 deletions.
11 changes: 1 addition & 10 deletions include/v8-internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ const int kApiSystemPointerSize = sizeof(void*);
const int kApiDoubleSize = sizeof(double);
const int kApiInt32Size = sizeof(int32_t);
const int kApiInt64Size = sizeof(int64_t);
const int kApiSizetSize = sizeof(size_t);

// Tag information for HeapObject.
const int kHeapObjectTag = 1;
Expand Down Expand Up @@ -232,10 +231,8 @@ class Internals {
kIsolateFastCCallCallerPcOffset + kApiSystemPointerSize;
static const int kIsolateCageBaseOffset =
kIsolateFastApiCallTargetOffset + kApiSystemPointerSize;
static const int kIsolateLongTaskStatsCounterOffset =
kIsolateCageBaseOffset + kApiSystemPointerSize;
static const int kIsolateStackGuardOffset =
kIsolateLongTaskStatsCounterOffset + kApiSizetSize;
kIsolateCageBaseOffset + kApiSystemPointerSize;
static const int kIsolateRootsOffset =
kIsolateStackGuardOffset + 7 * kApiSystemPointerSize;

Expand Down Expand Up @@ -368,12 +365,6 @@ class Internals {
return *reinterpret_cast<void* const*>(addr);
}

V8_INLINE static void IncrementLongTasksStatsCounter(v8::Isolate* isolate) {
internal::Address addr = reinterpret_cast<internal::Address>(isolate) +
kIsolateLongTaskStatsCounterOffset;
++(*reinterpret_cast<size_t*>(addr));
}

V8_INLINE static internal::Address* GetRoot(v8::Isolate* isolate, int index) {
internal::Address addr = reinterpret_cast<internal::Address>(isolate) +
kIsolateRootsOffset +
Expand Down
29 changes: 1 addition & 28 deletions include/v8-metrics.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
#ifndef V8_METRICS_H_
#define V8_METRICS_H_

#include "v8-internal.h" // NOLINT(build/include_directory)
#include "v8.h" // NOLINT(build/include_directory)
#include "v8.h" // NOLINT(build/include_directory)

namespace v8 {
namespace metrics {
Expand Down Expand Up @@ -184,32 +183,6 @@ class V8_EXPORT Recorder {
static ContextId GetContextId(Local<Context> context);
};

/**
* Experimental API intended for the LongTasks UKM (crbug.com/1173527).
* The Reset() method should be called at the start of a potential
* long task. The Get() method returns durations of V8 work that
* happened during the task.
*
* This API is experimental and may be removed/changed in the future.
*/
struct V8_EXPORT LongTaskStats {
/**
* Resets durations of V8 work for the new task.
*/
V8_INLINE static void Reset(Isolate* isolate) {
v8::internal::Internals::IncrementLongTasksStatsCounter(isolate);
}

/**
* Returns durations of V8 work that happened since the last Reset().
*/
static LongTaskStats Get(Isolate* isolate);

int64_t gc_full_atomic_wall_clock_duration_us = 0;
int64_t gc_full_incremental_wall_clock_duration_us = 0;
int64_t gc_young_wall_clock_duration_us = 0;
};

} // namespace metrics
} // namespace v8

Expand Down
5 changes: 0 additions & 5 deletions src/api/api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6371,11 +6371,6 @@ metrics::Recorder::ContextId metrics::Recorder::GetContextId(
handle(i_context->native_context(), isolate));
}

metrics::LongTaskStats metrics::LongTaskStats::Get(v8::Isolate* isolate) {
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
return *i_isolate->GetCurrentLongTaskStats();
}

namespace {
i::Address* GetSerializedDataFromFixedArray(i::Isolate* isolate,
i::FixedArray list, size_t index) {
Expand Down
7 changes: 0 additions & 7 deletions src/execution/isolate-data.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,6 @@ class IsolateData final {
V(kFastCCallCallerPCOffset, kSystemPointerSize) \
V(kFastApiCallTargetOffset, kSystemPointerSize) \
V(kCageBaseOffset, kSystemPointerSize) \
V(kLongTaskStatsCounterOffset, kSizetSize) \
V(kStackGuardOffset, StackGuard::kSizeInBytes) \
V(kRootsTableOffset, RootsTable::kEntriesCount* kSystemPointerSize) \
V(kExternalReferenceTableOffset, ExternalReferenceTable::kSizeInBytes) \
Expand Down Expand Up @@ -201,10 +200,6 @@ class IsolateData final {

Address cage_base_ = kNullAddress;

// Used for implementation of LongTaskStats. Counts the number of potential
// long tasks.
size_t long_task_stats_counter_ = 0;

// Fields related to the system and JS stack. In particular, this contains
// the stack limit used by stack checks in generated code.
StackGuard stack_guard_;
Expand Down Expand Up @@ -271,8 +266,6 @@ void IsolateData::AssertPredictableLayout() {
STATIC_ASSERT(offsetof(IsolateData, fast_api_call_target_) ==
kFastApiCallTargetOffset);
STATIC_ASSERT(offsetof(IsolateData, cage_base_) == kCageBaseOffset);
STATIC_ASSERT(offsetof(IsolateData, long_task_stats_counter_) ==
kLongTaskStatsCounterOffset);
STATIC_ASSERT(offsetof(IsolateData, stack_guard_) == kStackGuardOffset);
#ifdef V8_HEAP_SANDBOX
STATIC_ASSERT(offsetof(IsolateData, external_pointer_table_) ==
Expand Down
15 changes: 0 additions & 15 deletions src/execution/isolate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3057,9 +3057,6 @@ void Isolate::CheckIsolateLayout() {
Internals::kIsolateFastCCallCallerPcOffset);
CHECK_EQ(static_cast<int>(OFFSET_OF(Isolate, isolate_data_.cage_base_)),
Internals::kIsolateCageBaseOffset);
CHECK_EQ(static_cast<int>(
OFFSET_OF(Isolate, isolate_data_.long_task_stats_counter_)),
Internals::kIsolateLongTaskStatsCounterOffset);
CHECK_EQ(static_cast<int>(OFFSET_OF(Isolate, isolate_data_.stack_guard_)),
Internals::kIsolateStackGuardOffset);
CHECK_EQ(static_cast<int>(OFFSET_OF(Isolate, isolate_data_.roots_)),
Expand Down Expand Up @@ -5012,18 +5009,6 @@ MaybeLocal<v8::Context> Isolate::GetContextFromRecorderContextId(
return result->second.Get(reinterpret_cast<v8::Isolate*>(this));
}

void Isolate::UpdateLongTaskStats() {
if (last_long_task_stats_counter_ != isolate_data_.long_task_stats_counter_) {
last_long_task_stats_counter_ = isolate_data_.long_task_stats_counter_;
long_task_stats_ = v8::metrics::LongTaskStats{};
}
}

v8::metrics::LongTaskStats* Isolate::GetCurrentLongTaskStats() {
UpdateLongTaskStats();
return &long_task_stats_;
}

void Isolate::RemoveContextIdCallback(const v8::WeakCallbackInfo<void>& data) {
Isolate* isolate = reinterpret_cast<Isolate*>(data.GetIsolate());
uintptr_t context_id = reinterpret_cast<uintptr_t>(data.GetParameter());
Expand Down
6 changes: 0 additions & 6 deletions src/execution/isolate.h
Original file line number Diff line number Diff line change
Expand Up @@ -1740,9 +1740,6 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
MaybeLocal<v8::Context> GetContextFromRecorderContextId(
v8::metrics::Recorder::ContextId id);

void UpdateLongTaskStats();
v8::metrics::LongTaskStats* GetCurrentLongTaskStats();

LocalIsolate* main_thread_local_isolate() {
return main_thread_local_isolate_.get();
}
Expand Down Expand Up @@ -2107,9 +2104,6 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
Persistent<v8::Context, v8::CopyablePersistentTraits<v8::Context>>>
recorder_context_id_map_;

size_t last_long_task_stats_counter_ = 0;
v8::metrics::LongTaskStats long_task_stats_;

std::vector<Object> startup_object_cache_;

// Used during builtins compilation to build the builtins constants table,
Expand Down
15 changes: 0 additions & 15 deletions src/heap/gc-tracer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -87,15 +87,6 @@ GCTracer::Scope::~Scope() {
if (thread_kind_ == ThreadKind::kMain) {
DCHECK_EQ(tracer_->heap_->isolate()->thread_id(), ThreadId::Current());
tracer_->AddScopeSample(scope_, duration_ms);
if (scope_ == ScopeId::MC_INCREMENTAL ||
scope_ == ScopeId::MC_INCREMENTAL_START ||
scope_ == MC_INCREMENTAL_FINALIZE) {
auto* long_task_stats =
tracer_->heap_->isolate()->GetCurrentLongTaskStats();
long_task_stats->gc_full_incremental_wall_clock_duration_us +=
static_cast<int64_t>(duration_ms *
base::Time::kMicrosecondsPerMillisecond);
}
} else {
tracer_->AddScopeSampleBackground(scope_, duration_ms);
}
Expand Down Expand Up @@ -353,9 +344,6 @@ void GCTracer::Stop(GarbageCollector collector) {
AddAllocation(current_.end_time);

double duration = current_.end_time - current_.start_time;
int64_t duration_us =
static_cast<int64_t>(duration * base::Time::kMicrosecondsPerMillisecond);
auto* long_task_stats = heap_->isolate()->GetCurrentLongTaskStats();

switch (current_.type) {
case Event::SCAVENGER:
Expand All @@ -365,7 +353,6 @@ void GCTracer::Stop(GarbageCollector collector) {
recorded_minor_gcs_survived_.Push(
MakeBytesAndDuration(current_.survived_young_object_size, duration));
FetchBackgroundMinorGCCounters();
long_task_stats->gc_young_wall_clock_duration_us += duration_us;
break;
case Event::INCREMENTAL_MARK_COMPACTOR:
current_.incremental_marking_bytes = incremental_marking_bytes_;
Expand All @@ -385,7 +372,6 @@ void GCTracer::Stop(GarbageCollector collector) {
ResetIncrementalMarkingCounters();
combined_mark_compact_speed_cache_ = 0.0;
FetchBackgroundMarkCompactCounters();
long_task_stats->gc_full_atomic_wall_clock_duration_us += duration_us;
break;
case Event::MARK_COMPACTOR:
DCHECK_EQ(0u, current_.incremental_marking_bytes);
Expand All @@ -398,7 +384,6 @@ void GCTracer::Stop(GarbageCollector collector) {
ResetIncrementalMarkingCounters();
combined_mark_compact_speed_cache_ = 0.0;
FetchBackgroundMarkCompactCounters();
long_task_stats->gc_full_atomic_wall_clock_duration_us += duration_us;
break;
case Event::START:
UNREACHABLE();
Expand Down
79 changes: 6 additions & 73 deletions test/cctest/heap/test-heap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7383,90 +7383,23 @@ TEST(Regress10900) {
CcTest::CollectAllAvailableGarbage();
}

namespace {
void GenerateGarbage() {
const char* source =
"let roots = [];"
"for (let i = 0; i < 100; i++) roots.push(new Array(1000).fill(0));"
"roots.push(new Array(1000000).fill(0));"
"roots;";
CompileRun(source);
}

} // anonymous namespace

TEST(Regress11181) {
FLAG_always_compact = true;
CcTest::InitializeVM();
TracingFlags::runtime_stats.store(
v8::tracing::TracingCategoryObserver::ENABLED_BY_NATIVE,
std::memory_order_relaxed);
v8::HandleScope scope(CcTest::isolate());
GenerateGarbage();
const char* source =
"let roots = [];"
"for (let i = 0; i < 100; i++) roots.push(new Array(1000).fill(0));"
"roots.push(new Array(1000000).fill(0));"
"roots;";
CompileRun(source);
CcTest::CollectAllAvailableGarbage();
TracingFlags::runtime_stats.store(0, std::memory_order_relaxed);
}

TEST(LongTaskStatsFullAtomic) {
CcTest::InitializeVM();
v8::Isolate* isolate = CcTest::isolate();
v8::HandleScope scope(CcTest::isolate());
GenerateGarbage();
v8::metrics::LongTaskStats::Reset(isolate);
CHECK_EQ(0u, v8::metrics::LongTaskStats::Get(isolate)
.gc_full_atomic_wall_clock_duration_us);
for (int i = 0; i < 10; ++i) {
CcTest::CollectAllAvailableGarbage();
}
CHECK_LT(0u, v8::metrics::LongTaskStats::Get(isolate)
.gc_full_atomic_wall_clock_duration_us);
v8::metrics::LongTaskStats::Reset(isolate);
CHECK_EQ(0u, v8::metrics::LongTaskStats::Get(isolate)
.gc_full_atomic_wall_clock_duration_us);
}

TEST(LongTaskStatsFullIncremental) {
if (!FLAG_incremental_marking) return;
CcTest::InitializeVM();
v8::Isolate* isolate = CcTest::isolate();
v8::HandleScope scope(CcTest::isolate());
GenerateGarbage();
v8::metrics::LongTaskStats::Reset(isolate);
CHECK_EQ(0u, v8::metrics::LongTaskStats::Get(isolate)
.gc_full_incremental_wall_clock_duration_us);
for (int i = 0; i < 10; ++i) {
heap::SimulateIncrementalMarking(CcTest::heap());
CcTest::CollectAllAvailableGarbage();
}
CHECK_LT(0u, v8::metrics::LongTaskStats::Get(isolate)
.gc_full_incremental_wall_clock_duration_us);
v8::metrics::LongTaskStats::Reset(isolate);
CHECK_EQ(0u, v8::metrics::LongTaskStats::Get(isolate)
.gc_full_incremental_wall_clock_duration_us);
}

TEST(LongTaskStatsYoung) {
if (FLAG_single_generation) return;
CcTest::InitializeVM();
v8::Isolate* isolate = CcTest::isolate();
v8::HandleScope scope(CcTest::isolate());
GenerateGarbage();
v8::metrics::LongTaskStats::Reset(isolate);
CHECK_EQ(
0u,
v8::metrics::LongTaskStats::Get(isolate).gc_young_wall_clock_duration_us);
for (int i = 0; i < 10; ++i) {
CcTest::CollectGarbage(NEW_SPACE);
}
CHECK_LT(
0u,
v8::metrics::LongTaskStats::Get(isolate).gc_young_wall_clock_duration_us);
v8::metrics::LongTaskStats::Reset(isolate);
CHECK_EQ(
0u,
v8::metrics::LongTaskStats::Get(isolate).gc_young_wall_clock_duration_us);
}

} // namespace heap
} // namespace internal
} // namespace v8
Expand Down

0 comments on commit e408cc2

Please sign in to comment.