From 245a5b38e7ed136e3e88ffc13a390ff46e63376e Mon Sep 17 00:00:00 2001 From: Andrew Comminos Date: Tue, 21 May 2019 17:06:41 -0700 Subject: [PATCH] [cpu-profiler] Remove redundant record_samples option Now that we support a max_samples parameter, it isn't actually necessary to have a record_samples flag (as it can just be modeled by 0). Change-Id: I578ecc9f6ee73ecbe1f93d0d04ee8028a9a2716d Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1611015 Commit-Queue: Andrew Comminos Reviewed-by: Alexei Filippov Reviewed-by: Ulan Degenbaev Cr-Commit-Position: refs/heads/master@{#61717} --- include/v8-profiler.h | 5 --- src/api/api.cc | 6 ++- src/profiler/profile-generator.cc | 2 +- src/profiler/tracing-cpu-profiler.cc | 2 +- test/cctest/test-cpu-profiler.cc | 60 ++++++++++++--------------- test/cctest/test-profile-generator.cc | 2 +- 6 files changed, 33 insertions(+), 44 deletions(-) diff --git a/include/v8-profiler.h b/include/v8-profiler.h index 62f9d4354f..46d3eb8aa4 100644 --- a/include/v8-profiler.h +++ b/include/v8-profiler.h @@ -317,7 +317,6 @@ class V8_EXPORT CpuProfilingOptions { /** * \param mode Type of computation of stack frame line numbers. - * \param record_samples Whether samples should be logged in the profile. * \param max_samples The maximum number of samples that should be recorded by * the profiler. Samples obtained after this limit will be * discarded. @@ -330,22 +329,18 @@ class V8_EXPORT CpuProfilingOptions { * the profiler's sampling interval. */ CpuProfilingOptions(CpuProfilingMode mode = kLeafNodeLineNumbers, - bool record_samples = false, unsigned max_samples = kNoSampleLimit, int sampling_interval_us = 0) : mode_(mode), - record_samples_(record_samples), max_samples_(max_samples), sampling_interval_us_(sampling_interval_us) {} CpuProfilingMode mode() const { return mode_; } - bool record_samples() const { return record_samples_; } unsigned max_samples() const { return max_samples_; } int sampling_interval_us() const { return sampling_interval_us_; } private: CpuProfilingMode mode_; - bool record_samples_; unsigned max_samples_; int sampling_interval_us_; }; diff --git a/src/api/api.cc b/src/api/api.cc index ca0707f96f..3e8cba81c2 100644 --- a/src/api/api.cc +++ b/src/api/api.cc @@ -9761,14 +9761,16 @@ void CpuProfiler::StartProfiling(Local title, } void CpuProfiler::StartProfiling(Local title, bool record_samples) { - CpuProfilingOptions options(kLeafNodeLineNumbers, record_samples); + CpuProfilingOptions options( + kLeafNodeLineNumbers, + record_samples ? CpuProfilingOptions::kNoSampleLimit : 0); reinterpret_cast(this)->StartProfiling( *Utils::OpenHandle(*title), options); } void CpuProfiler::StartProfiling(Local title, CpuProfilingMode mode, bool record_samples, unsigned max_samples) { - CpuProfilingOptions options(mode, record_samples, max_samples); + CpuProfilingOptions options(mode, record_samples ? max_samples : 0); reinterpret_cast(this)->StartProfiling( *Utils::OpenHandle(*title), options); } diff --git a/src/profiler/profile-generator.cc b/src/profiler/profile-generator.cc index 251c1527e9..354bd2ff44 100644 --- a/src/profiler/profile-generator.cc +++ b/src/profiler/profile-generator.cc @@ -516,7 +516,7 @@ void CpuProfile::AddPath(base::TimeTicks timestamp, top_down_.AddPathFromEnd(path, src_line, update_stats, options_.mode()); bool should_record_sample = - options_.record_samples() && !timestamp.IsNull() && + !timestamp.IsNull() && (options_.max_samples() == CpuProfilingOptions::kNoSampleLimit || samples_.size() < options_.max_samples()); diff --git a/src/profiler/tracing-cpu-profiler.cc b/src/profiler/tracing-cpu-profiler.cc index 917e978006..ea48b51a81 100644 --- a/src/profiler/tracing-cpu-profiler.cc +++ b/src/profiler/tracing-cpu-profiler.cc @@ -56,7 +56,7 @@ void TracingCpuProfilerImpl::StartProfiling() { profiler_.reset(new CpuProfiler(isolate_, kDebugNaming)); profiler_->set_sampling_interval( base::TimeDelta::FromMicroseconds(sampling_interval_us)); - profiler_->StartProfiling("", {kLeafNodeLineNumbers, true}); + profiler_->StartProfiling("", {kLeafNodeLineNumbers}); } void TracingCpuProfilerImpl::StopProfiling() { diff --git a/test/cctest/test-cpu-profiler.cc b/test/cctest/test-cpu-profiler.cc index 1c5327b523..1eb4c80c34 100644 --- a/test/cctest/test-cpu-profiler.cc +++ b/test/cctest/test-cpu-profiler.cc @@ -439,7 +439,6 @@ class ProfilerHelper { v8::CpuProfile* Run( v8::Local function, v8::Local argv[], int argc, unsigned min_js_samples = 0, unsigned min_external_samples = 0, - bool collect_samples = false, ProfilingMode mode = ProfilingMode::kLeafNodeLineNumbers, unsigned max_samples = CpuProfilingOptions::kNoSampleLimit); @@ -454,12 +453,11 @@ v8::CpuProfile* ProfilerHelper::Run(v8::Local function, v8::Local argv[], int argc, unsigned min_js_samples, unsigned min_external_samples, - bool collect_samples, ProfilingMode mode, - unsigned max_samples) { + ProfilingMode mode, unsigned max_samples) { v8::Local profile_name = v8_str("my_profile"); profiler_->SetSamplingInterval(100); - profiler_->StartProfiling(profile_name, {mode, collect_samples, max_samples}); + profiler_->StartProfiling(profile_name, {mode, max_samples}); v8::internal::CpuProfiler* iprofiler = reinterpret_cast(profiler_); @@ -667,11 +665,11 @@ TEST(CollectCpuProfileCallerLineNumbers) { v8::Local args[] = { v8::Integer::New(env->GetIsolate(), profiling_interval_ms)}; ProfilerHelper helper(env.local()); - helper.Run(function, args, arraysize(args), 1000, 0, false, - v8::CpuProfilingMode::kCallerLineNumbers); + helper.Run(function, args, arraysize(args), 1000, 0, + v8::CpuProfilingMode::kCallerLineNumbers, 0); v8::CpuProfile* profile = - helper.Run(function, args, arraysize(args), 1000, 0, false, - v8::CpuProfilingMode::kCallerLineNumbers); + helper.Run(function, args, arraysize(args), 1000, 0, + v8::CpuProfilingMode::kCallerLineNumbers, 0); const v8::CpuProfileNode* root = profile->GetTopDownRoot(); const v8::CpuProfileNode* start_node = GetChild(root, {"start", 27}); @@ -753,7 +751,7 @@ TEST(CollectCpuProfileSamples) { v8::Integer::New(env->GetIsolate(), profiling_interval_ms)}; ProfilerHelper helper(env.local()); v8::CpuProfile* profile = - helper.Run(function, args, arraysize(args), 1000, 0, true); + helper.Run(function, args, arraysize(args), 1000, 0); CHECK_LE(200, profile->GetSamplesCount()); uint64_t end_time = profile->GetEndTime(); @@ -2858,7 +2856,7 @@ TEST(NativeFrameStackTrace) { ProfilerHelper helper(env); - v8::CpuProfile* profile = helper.Run(function, nullptr, 0, 100, 0, true); + v8::CpuProfile* profile = helper.Run(function, nullptr, 0, 100, 0); // Count the fraction of samples landing in 'jsFunction' (valid stack) // vs '(program)' (no stack captured). @@ -2966,7 +2964,7 @@ TEST(MultipleProfilersSampleIndependently) { std::unique_ptr slow_profiler( new CpuProfiler(CcTest::i_isolate())); slow_profiler->set_sampling_interval(base::TimeDelta::FromSeconds(1)); - slow_profiler->StartProfiling("1", {kLeafNodeLineNumbers, true}); + slow_profiler->StartProfiling("1", {kLeafNodeLineNumbers}); CompileRun(R"( function start() { @@ -2979,7 +2977,7 @@ TEST(MultipleProfilersSampleIndependently) { )"); v8::Local function = GetFunction(env.local(), "start"); ProfilerHelper helper(env.local()); - v8::CpuProfile* profile = helper.Run(function, nullptr, 0, 100, 0, true); + v8::CpuProfile* profile = helper.Run(function, nullptr, 0, 100, 0); auto slow_profile = slow_profiler->StopProfiling("1"); CHECK_GT(profile->GetSamplesCount(), slow_profile->samples_count()); @@ -3045,7 +3043,7 @@ TEST(FastStopProfiling) { std::unique_ptr profiler(new CpuProfiler(CcTest::i_isolate())); profiler->set_sampling_interval(kLongInterval); - profiler->StartProfiling("", {kLeafNodeLineNumbers, true}); + profiler->StartProfiling("", {kLeafNodeLineNumbers}); v8::Platform* platform = v8::internal::V8::GetCurrentPlatform(); double start = platform->CurrentClockTimeMillis(); @@ -3169,7 +3167,7 @@ TEST(SampleLimit) { v8::Local function = GetFunction(env.local(), "start"); ProfilerHelper helper(env.local()); v8::CpuProfile* profile = - helper.Run(function, nullptr, 0, 100, 0, true, + helper.Run(function, nullptr, 0, 100, 0, v8::CpuProfilingMode::kLeafNodeLineNumbers, 50); CHECK_EQ(profile->GetSamplesCount(), 50); @@ -3191,7 +3189,7 @@ TEST(ProflilerSubsampling) { // Create a new CpuProfile that wants samples at 8us. CpuProfile profile(&profiler, "", - {v8::CpuProfilingMode::kLeafNodeLineNumbers, true, + {v8::CpuProfilingMode::kLeafNodeLineNumbers, v8::CpuProfilingOptions::kNoSampleLimit, 8}); // Verify that the first sample is always included. CHECK(profile.CheckSubsample(base::TimeDelta::FromMicroseconds(10))); @@ -3239,24 +3237,22 @@ TEST(DynamicResampling) { // Add a 10us profiler, verify that the base sampling interval is as high as // possible (10us). profiles->StartProfiling("10us", - {v8::CpuProfilingMode::kLeafNodeLineNumbers, true, + {v8::CpuProfilingMode::kLeafNodeLineNumbers, v8::CpuProfilingOptions::kNoSampleLimit, 10}); CHECK_EQ(profiles->GetCommonSamplingInterval(), base::TimeDelta::FromMicroseconds(10)); // Add a 5us profiler, verify that the base sampling interval is as high as // possible given a 10us and 5us profiler (5us). - profiles->StartProfiling("5us", - {v8::CpuProfilingMode::kLeafNodeLineNumbers, true, - v8::CpuProfilingOptions::kNoSampleLimit, 5}); + profiles->StartProfiling("5us", {v8::CpuProfilingMode::kLeafNodeLineNumbers, + v8::CpuProfilingOptions::kNoSampleLimit, 5}); CHECK_EQ(profiles->GetCommonSamplingInterval(), base::TimeDelta::FromMicroseconds(5)); // Add a 3us profiler, verify that the base sampling interval is 1us (due to // coprime intervals). - profiles->StartProfiling("3us", - {v8::CpuProfilingMode::kLeafNodeLineNumbers, true, - v8::CpuProfilingOptions::kNoSampleLimit, 3}); + profiles->StartProfiling("3us", {v8::CpuProfilingMode::kLeafNodeLineNumbers, + v8::CpuProfilingOptions::kNoSampleLimit, 3}); CHECK_EQ(profiles->GetCommonSamplingInterval(), base::TimeDelta::FromMicroseconds(1)); @@ -3297,24 +3293,21 @@ TEST(DynamicResamplingWithBaseInterval) { // Add a profiler with an unset sampling interval, verify that the common // sampling interval is equal to the base. - profiles->StartProfiling("unset", - {v8::CpuProfilingMode::kLeafNodeLineNumbers, true, - v8::CpuProfilingOptions::kNoSampleLimit}); + profiles->StartProfiling("unset", {v8::CpuProfilingMode::kLeafNodeLineNumbers, + v8::CpuProfilingOptions::kNoSampleLimit}); CHECK_EQ(profiles->GetCommonSamplingInterval(), base::TimeDelta::FromMicroseconds(7)); profiles->StopProfiling("unset"); // Adding a 8us sampling interval rounds to a 14us base interval. - profiles->StartProfiling("8us", - {v8::CpuProfilingMode::kLeafNodeLineNumbers, true, - v8::CpuProfilingOptions::kNoSampleLimit, 8}); + profiles->StartProfiling("8us", {v8::CpuProfilingMode::kLeafNodeLineNumbers, + v8::CpuProfilingOptions::kNoSampleLimit, 8}); CHECK_EQ(profiles->GetCommonSamplingInterval(), base::TimeDelta::FromMicroseconds(14)); // Adding a 4us sampling interval should cause a lowering to a 7us interval. - profiles->StartProfiling("4us", - {v8::CpuProfilingMode::kLeafNodeLineNumbers, true, - v8::CpuProfilingOptions::kNoSampleLimit, 4}); + profiles->StartProfiling("4us", {v8::CpuProfilingMode::kLeafNodeLineNumbers, + v8::CpuProfilingOptions::kNoSampleLimit, 4}); CHECK_EQ(profiles->GetCommonSamplingInterval(), base::TimeDelta::FromMicroseconds(7)); @@ -3332,9 +3325,8 @@ TEST(DynamicResamplingWithBaseInterval) { // A sampling interval of 0us should enforce all profiles to have a sampling // interval of 0us (the only multiple of 0). profiler.set_sampling_interval(base::TimeDelta::FromMicroseconds(0)); - profiles->StartProfiling("5us", - {v8::CpuProfilingMode::kLeafNodeLineNumbers, true, - v8::CpuProfilingOptions::kNoSampleLimit, 5}); + profiles->StartProfiling("5us", {v8::CpuProfilingMode::kLeafNodeLineNumbers, + v8::CpuProfilingOptions::kNoSampleLimit, 5}); CHECK_EQ(profiles->GetCommonSamplingInterval(), base::TimeDelta::FromMicroseconds(0)); profiles->StopProfiling("5us"); diff --git a/test/cctest/test-profile-generator.cc b/test/cctest/test-profile-generator.cc index 13005f0924..d0a34d42b0 100644 --- a/test/cctest/test-profile-generator.cc +++ b/test/cctest/test-profile-generator.cc @@ -448,7 +448,7 @@ TEST(SampleIds) { CpuProfilesCollection profiles(isolate); CpuProfiler profiler(isolate); profiles.set_cpu_profiler(&profiler); - profiles.StartProfiling("", {CpuProfilingMode::kLeafNodeLineNumbers, true}); + profiles.StartProfiling("", {CpuProfilingMode::kLeafNodeLineNumbers}); ProfileGenerator generator(&profiles); CodeEntry* entry1 = new CodeEntry(i::Logger::FUNCTION_TAG, "aaa"); CodeEntry* entry2 = new CodeEntry(i::Logger::FUNCTION_TAG, "bbb");