diff --git a/components/heap_profiling/in_process/BUILD.gn b/components/heap_profiling/in_process/BUILD.gn index d50517b4c2c65..b4638d8b8a50c 100644 --- a/components/heap_profiling/in_process/BUILD.gn +++ b/components/heap_profiling/in_process/BUILD.gn @@ -55,7 +55,10 @@ source_set("unit_tests") { # HeapProfilerController's dependencies are not compiled on iOS unless # use_allocator_shim is true. if (!is_ios || use_allocator_shim) { - sources = [ "heap_profiler_controller_unittest.cc" ] + sources = [ + "heap_profiler_controller_unittest.cc", + "heap_profiler_parameters_unittest.cc", + ] deps = [ ":in_process", ":mojom", diff --git a/components/heap_profiling/in_process/browser_process_snapshot_controller.cc b/components/heap_profiling/in_process/browser_process_snapshot_controller.cc index ebca07ba3b159..07e90e8bb4c21 100644 --- a/components/heap_profiling/in_process/browser_process_snapshot_controller.cc +++ b/components/heap_profiling/in_process/browser_process_snapshot_controller.cc @@ -7,12 +7,14 @@ #include <memory> #include <utility> -#include "base/check.h" +#include "base/check_op.h" #include "base/containers/flat_map.h" #include "base/functional/bind.h" #include "base/functional/callback.h" #include "base/memory/scoped_refptr.h" #include "base/memory/weak_ptr.h" +#include "base/metrics/field_trial_params.h" +#include "base/notreached.h" #include "base/rand_util.h" #include "base/sequence_checker.h" #include "base/task/sequenced_task_runner.h" @@ -97,8 +99,25 @@ void BrowserProcessSnapshotController::TakeSnapshotsOnSnapshotSequence() { // processes to measure. continue; } - const int snapshot_probability_pct = - GetSnapshotProbabilityForProcess(process_type); + int snapshot_probability_pct; + switch (process_type) { + case sampling_profiler::ProfilerProcessType::kGpu: + snapshot_probability_pct = kGpuSnapshotProbability.Get(); + break; + case sampling_profiler::ProfilerProcessType::kNetworkService: + snapshot_probability_pct = kNetworkSnapshotProbability.Get(); + break; + case sampling_profiler::ProfilerProcessType::kRenderer: + snapshot_probability_pct = kRendererSnapshotProbability.Get(); + break; + case sampling_profiler::ProfilerProcessType::kUtility: + snapshot_probability_pct = kUtilitySnapshotProbability.Get(); + break; + default: + NOTREACHED(); + } + CHECK_GE(snapshot_probability_pct, 0); + CHECK_LE(snapshot_probability_pct, 100); if (snapshot_probability_pct == 0) { // No need to test each process since none will be chosen. continue; diff --git a/components/heap_profiling/in_process/heap_profiler_controller.cc b/components/heap_profiling/in_process/heap_profiler_controller.cc index 25c9623b78f93..b3dc51b2b5549 100644 --- a/components/heap_profiling/in_process/heap_profiler_controller.cc +++ b/components/heap_profiling/in_process/heap_profiler_controller.cc @@ -17,12 +17,10 @@ #include "base/allocator/dispatcher/reentry_guard.h" #include "base/check_op.h" #include "base/command_line.h" -#include "base/feature_list.h" #include "base/functional/bind.h" #include "base/functional/callback.h" #include "base/memory/scoped_refptr.h" #include "base/memory/weak_ptr.h" -#include "base/metrics/field_trial_params.h" #include "base/metrics/histogram_functions.h" #include "base/metrics/metrics_hashes.h" #include "base/notreached.h" @@ -45,7 +43,6 @@ #include "components/metrics/call_stacks/call_stack_profile_builder.h" #include "components/sampling_profiler/process_type.h" #include "components/services/heap_profiling/public/cpp/merge_samples.h" -#include "components/variations/variations_switches.h" #include "components/version_info/channel.h" #include "third_party/abseil-cpp/absl/cleanup/cleanup.h" @@ -110,18 +107,19 @@ std::string ProcessHistogramName(std::string_view base_name, } } -double GetChannelProbability(version_info::Channel channel) { +double GetChannelProbability(version_info::Channel channel, + const HeapProfilerParameters& params) { switch (channel) { case version_info::Channel::STABLE: case version_info::Channel::UNKNOWN: // If the channel can't be determined, treat it as `stable` for safety. // Don't disable heap profiling completely so that developers can still // enable it with --enable-feature flags. - return kStableProbability.Get(); + return params.stable_probability; case version_info::Channel::BETA: case version_info::Channel::DEV: case version_info::Channel::CANARY: - return kNonStableProbability.Get(); + return params.nonstable_probability; } NOTREACHED(); } @@ -142,19 +140,15 @@ std::pair<bool, std::optional<std::string>> DecideIfCollectionIsEnabled( return {is_enabled, std::nullopt}; } - // Never profile during benchmarking. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - variations::switches::kEnableBenchmarking)) { - return {false, std::nullopt}; - } - - if (!base::FeatureList::IsEnabled(kHeapProfilerReporting)) { - return {false, std::nullopt}; - } - // Randomly determine whether profiling is enabled. + HeapProfilerParameters params = + GetHeapProfilerParametersForProcess(process_type); + if (!params.is_supported) { + return {false, std::nullopt}; + } + const double seed = base::RandDouble(); - const double probability = GetChannelProbability(channel); + const double probability = GetChannelProbability(channel, params); if (seed < probability) { return {true, "Enabled"}; } @@ -263,9 +257,13 @@ bool HeapProfilerController::StartIfEnabled() { if (!profiling_enabled_) { return false; } - const size_t sampling_rate_bytes = GetSamplingRateForProcess(process_type_); - if (sampling_rate_bytes > 0) { - base::SamplingHeapProfiler::Get()->SetSamplingInterval(sampling_rate_bytes); + HeapProfilerParameters profiler_params = + GetHeapProfilerParametersForProcess(process_type_); + // DecideIfCollectionIsEnabled() should return false if not supported. + DCHECK(profiler_params.is_supported); + if (profiler_params.sampling_rate_bytes > 0) { + base::SamplingHeapProfiler::Get()->SetSamplingInterval( + profiler_params.sampling_rate_bytes); } base::SamplingHeapProfiler::Get()->Start(); @@ -274,10 +272,9 @@ bool HeapProfilerController::StartIfEnabled() { return true; } - const base::TimeDelta collection_interval = kCollectionInterval.Get(); - CHECK(collection_interval.is_positive()); + DCHECK(profiler_params.collection_interval.is_positive()); SnapshotParams params( - collection_interval, + profiler_params.collection_interval, /*use_random_interval=*/!suppress_randomness_for_testing_, stopped_, process_type_, creation_time_, std::move(on_first_snapshot_callback_)); params.trigger_child_process_snapshot_closure = base::BindRepeating( @@ -371,7 +368,7 @@ void HeapProfilerController::AppendCommandLineSwitchInternal( BrowserProcessSnapshotController* snapshot_controller) { CHECK_NE(child_process_type, ProcessType::kBrowser); if (snapshot_controller && - GetSnapshotProbabilityForProcess(child_process_type) > 0) { + GetHeapProfilerParametersForProcess(child_process_type).is_supported) { command_line->AppendSwitch(switches::kSubprocessHeapProfiling); snapshot_controller->BindRemoteForChildProcess(child_process_id, child_process_type); diff --git a/components/heap_profiling/in_process/heap_profiler_controller_unittest.cc b/components/heap_profiling/in_process/heap_profiler_controller_unittest.cc index aa8605e656f27..9e304e2be173f 100644 --- a/components/heap_profiling/in_process/heap_profiler_controller_unittest.cc +++ b/components/heap_profiling/in_process/heap_profiler_controller_unittest.cc @@ -19,12 +19,16 @@ #include "base/auto_reset.h" #include "base/barrier_closure.h" #include "base/command_line.h" +#include "base/containers/contains.h" +#include "base/containers/enum_set.h" #include "base/functional/bind.h" #include "base/functional/callback.h" #include "base/functional/callback_helpers.h" +#include "base/json/json_writer.h" #include "base/memory/scoped_refptr.h" #include "base/metrics/field_trial_params.h" #include "base/metrics/metrics_hashes.h" +#include "base/notreached.h" #include "base/process/launch.h" #include "base/process/process.h" #include "base/sampling_heap_profiler/poisson_allocation_sampler.h" @@ -42,6 +46,7 @@ #include "base/test/test_timeouts.h" #include "base/time/time.h" #include "base/types/optional_util.h" +#include "base/values.h" #include "build/build_config.h" #include "components/heap_profiling/in_process/browser_process_snapshot_controller.h" #include "components/heap_profiling/in_process/child_process_snapshot_controller.h" @@ -113,12 +118,21 @@ namespace { #define ENABLE_MULTIPROCESS_TESTS 1 #endif +using FeatureRef = base::test::FeatureRef; +using FeatureRefAndParams = base::test::FeatureRefAndParams; +using ProcessType = sampling_profiler::ProfilerProcessType; +using ProcessTypeSet = + base::EnumSet<ProcessType, ProcessType::kUnknown, ProcessType::kMax>; +using ProfileCollectorCallback = + base::RepeatingCallback<void(base::TimeTicks, metrics::SampledProfile)>; using base::allocator::dispatcher::AllocationNotificationData; using base::allocator::dispatcher::AllocationSubsystem; using base::allocator::dispatcher::FreeNotificationData; -using base::test::FeatureRef; -using base::test::FeatureRefAndParams; -using sampling_profiler::ProfilerProcessType; +using ScopedMuteHookedSamplesForTesting = + base::PoissonAllocationSampler::ScopedMuteHookedSamplesForTesting; +using ScopedSuppressRandomnessForTesting = + base::PoissonAllocationSampler::ScopedSuppressRandomnessForTesting; + using ::testing::_; using ::testing::AllOf; using ::testing::Conditional; @@ -131,13 +145,6 @@ using ::testing::Property; using ::testing::ResultOf; using ::testing::UnorderedElementsAreArray; -using ProfileCollectorCallback = - base::RepeatingCallback<void(base::TimeTicks, metrics::SampledProfile)>; -using ScopedMuteHookedSamplesForTesting = - base::PoissonAllocationSampler::ScopedMuteHookedSamplesForTesting; -using ScopedSuppressRandomnessForTesting = - base::PoissonAllocationSampler::ScopedSuppressRandomnessForTesting; - constexpr size_t kSamplingRate = 1024; constexpr size_t kAllocationSize = 42 * kSamplingRate; @@ -393,9 +400,8 @@ class MultiprocessTestChild final : public mojom::TestConnector, // Start the heap profiler and wait for TakeSnapshot() messages from the // parent. - HeapProfilerController controller( - version_info::Channel::STABLE, - static_cast<ProfilerProcessType>(process_type)); + HeapProfilerController controller(version_info::Channel::STABLE, + static_cast<ProcessType>(process_type)); controller.SuppressRandomnessForTesting(); ASSERT_TRUE(controller.IsEnabled()); controller.StartIfEnabled(); @@ -517,7 +523,7 @@ class MultiprocessTestParent { // `should_profile` is false, simulate the embedder refusing to profile the // child process. void LaunchTestChild(HeapProfilerController* controller, - ProfilerProcessType process_type, + ProcessType process_type, int num_allocations, bool should_profile) { // `should_profile` will apply during next call to BindTestConnector(). @@ -599,13 +605,14 @@ struct FeatureTestParams { }; // Whether HeapProfilerReporting is enabled. bool feature_enabled = true; + const ProcessTypeSet supported_processes; ChannelParams stable; ChannelParams nonstable; // Probabilities for snapshotting child processes. - int gpu_snapshot_prob = 0; - int network_snapshot_prob = 0; - int renderer_snapshot_prob = 0; - int utility_snapshot_prob = 0; + int gpu_snapshot_prob = 100; + int network_snapshot_prob = 100; + int renderer_snapshot_prob = 100; + int utility_snapshot_prob = 100; base::FieldTrialParams ToFieldTrialParams() const; @@ -616,32 +623,58 @@ struct FeatureTestParams { // Converts the test params to field trial parameters for the // HeapProfilerReporting feature. base::FieldTrialParams FeatureTestParams::ToFieldTrialParams() const { - base::FieldTrialParams field_trial_params; + base::FieldTrialParams field_trial_params{ + {"gpu-prob-pct", base::NumberToString(gpu_snapshot_prob)}, + {"network-prob-pct", base::NumberToString(network_snapshot_prob)}, + {"renderer-prob-pct", base::NumberToString(renderer_snapshot_prob)}, + {"utility-prob-pct", base::NumberToString(utility_snapshot_prob)}, + }; - // Global parameters. - field_trial_params["stable-probability"] = - base::NumberToString(stable.probability); - field_trial_params["nonstable-probability"] = - base::NumberToString(nonstable.probability); + // Add the default params. + base::Value::Dict dict; + if (!supported_processes.empty()) { + // Explicitly disable profiling by default, so that only the processes + // given in `supported_processes` will be enabled. + dict.Set("is-supported", false); + } + dict.Set("stable-probability", stable.probability); + dict.Set("nonstable-probability", nonstable.probability); + dict.Set("sampling-rate-bytes", static_cast<int>(kSamplingRate)); + std::string param_string; + base::JSONWriter::WriteWithOptions( + dict, base::JSONWriter::OPTIONS_PRETTY_PRINT, ¶m_string); + field_trial_params["default-params"] = param_string; - // Per-process parameters. - field_trial_params["browser-sampling-rate-bytes"] = - base::NumberToString(kSamplingRate); - field_trial_params["gpu-sampling-rate-bytes"] = - base::NumberToString(kSamplingRate); - field_trial_params["gpu-prob-pct"] = base::NumberToString(gpu_snapshot_prob); - field_trial_params["network-sampling-rate-bytes"] = - base::NumberToString(kSamplingRate); - field_trial_params["network-prob-pct"] = - base::NumberToString(network_snapshot_prob); - field_trial_params["renderer-sampling-rate-bytes"] = - base::NumberToString(kSamplingRate); - field_trial_params["renderer-prob-pct"] = - base::NumberToString(renderer_snapshot_prob); - field_trial_params["utility-sampling-rate-bytes"] = - base::NumberToString(kSamplingRate); - field_trial_params["utility-prob-pct"] = - base::NumberToString(utility_snapshot_prob); + // Add a field trial param that enables each process type in + // `supported_processes`. + base::Value::Dict is_supported_dict; + is_supported_dict.Set("is-supported", true); + std::string is_supported_string; + base::JSONWriter::WriteWithOptions(is_supported_dict, + base::JSONWriter::OPTIONS_PRETTY_PRINT, + &is_supported_string); + + for (ProcessType process_type : supported_processes) { + switch (process_type) { + case ProcessType::kBrowser: + field_trial_params["browser-process-params"] = is_supported_string; + break; + case ProcessType::kRenderer: + field_trial_params["renderer-process-params"] = is_supported_string; + break; + case ProcessType::kGpu: + field_trial_params["gpu-process-params"] = is_supported_string; + break; + case ProcessType::kUtility: + field_trial_params["utility-process-params"] = is_supported_string; + break; + case ProcessType::kNetworkService: + field_trial_params["network-process-params"] = is_supported_string; + break; + default: + NOTREACHED(); + } + } return field_trial_params; } @@ -729,18 +762,18 @@ class HeapProfilerControllerTest // The test must call StartIfEnabled() after this to start profiling. void CreateHeapProfiler( version_info::Channel channel, - ProfilerProcessType process_type, + ProcessType process_type, bool expect_enabled, base::OnceClosure first_snapshot_callback = base::DoNothing(), ProfileCollectorCallback collector_callback = base::DoNothing()) { ASSERT_FALSE(controller_) << "CreateHeapProfiler called twice"; switch (process_type) { - case ProfilerProcessType::kBrowser: + case ProcessType::kBrowser: expected_process_ = metrics::Process::BROWSER_PROCESS; metrics::CallStackProfileBuilder::SetBrowserProcessReceiverCallback( std::move(collector_callback)); break; - case ProfilerProcessType::kUtility: + case ProcessType::kUtility: expected_process_ = metrics::Process::UTILITY_PROCESS; metrics::CallStackProfileBuilder:: SetParentProfileCollectorForChildProcess( @@ -772,7 +805,7 @@ class HeapProfilerControllerTest // profiling. void StartHeapProfiling( version_info::Channel channel, - ProfilerProcessType process_type, + ProcessType process_type, bool expect_enabled, base::OnceClosure first_snapshot_callback = base::DoNothing(), ProfileCollectorCallback collector_callback = base::DoNothing()) { @@ -874,8 +907,7 @@ TEST_P(HeapProfilerControllerTest, ProfileCollectionsScheduler) { ++profile_count; }; - StartHeapProfiling(version_info::Channel::STABLE, - ProfilerProcessType::kBrowser, + StartHeapProfiling(version_info::Channel::STABLE, ProcessType::kBrowser, /*expect_enabled=*/true, /*first_snapshot_callback=*/base::DoNothing(), base::BindLambdaForTesting(check_profile)); @@ -905,8 +937,7 @@ TEST_P(HeapProfilerControllerTest, ProfileCollectionsScheduler) { TEST_P(HeapProfilerControllerTest, UnhandledProcess) { // Starting the heap profiler in an unhandled process type should safely do // nothing. - StartHeapProfiling(version_info::Channel::STABLE, - ProfilerProcessType::kUnknown, + StartHeapProfiling(version_info::Channel::STABLE, ProcessType::kUnknown, /*expect_enabled=*/false); // The Enabled summary histogram should not be logged for unsupported // processes, because they're not included in the per-process histograms that @@ -918,10 +949,10 @@ TEST_P(HeapProfilerControllerTest, EmptyProfile) { // Should save an empty profile even though no memory is allocated. ScopedCallbacks callbacks = CreateScopedCallbacks( /*expect_take_snapshot=*/true, /*expect_sampled_profile=*/true); - StartHeapProfiling( - version_info::Channel::STABLE, ProfilerProcessType::kBrowser, - /*expect_enabled=*/true, callbacks.first_snapshot_callback(), - callbacks.collector_callback()); + StartHeapProfiling(version_info::Channel::STABLE, ProcessType::kBrowser, + /*expect_enabled=*/true, + callbacks.first_snapshot_callback(), + callbacks.collector_callback()); task_env().RunUntilQuit(); EXPECT_TRUE(sample_received_); } @@ -972,9 +1003,8 @@ TEST_P(HeapProfilerControllerChannelTest, StableChannel) { ScopedCallbacks callbacks = CreateScopedCallbacks( /*expect_take_snapshot=*/profiling_enabled, GetParam().stable.expect_browser_sample); - StartHeapProfiling(version_info::Channel::STABLE, - ProfilerProcessType::kBrowser, profiling_enabled, - callbacks.first_snapshot_callback(), + StartHeapProfiling(version_info::Channel::STABLE, ProcessType::kBrowser, + profiling_enabled, callbacks.first_snapshot_callback(), callbacks.collector_callback()); histogram_tester_.ExpectUniqueSample( "HeapProfiling.InProcess.Enabled.Browser", profiling_enabled, 1); @@ -990,9 +1020,8 @@ TEST_P(HeapProfilerControllerChannelTest, CanaryChannel) { ScopedCallbacks callbacks = CreateScopedCallbacks( /*expect_take_snapshot=*/profiling_enabled, GetParam().nonstable.expect_browser_sample); - StartHeapProfiling(version_info::Channel::CANARY, - ProfilerProcessType::kBrowser, profiling_enabled, - callbacks.first_snapshot_callback(), + StartHeapProfiling(version_info::Channel::CANARY, ProcessType::kBrowser, + profiling_enabled, callbacks.first_snapshot_callback(), callbacks.collector_callback()); histogram_tester_.ExpectUniqueSample( "HeapProfiling.InProcess.Enabled.Browser", profiling_enabled, 1); @@ -1010,9 +1039,8 @@ TEST_P(HeapProfilerControllerChannelTest, UnknownChannel) { ScopedCallbacks callbacks = CreateScopedCallbacks( /*expect_take_snapshot=*/profiling_enabled, GetParam().stable.expect_browser_sample); - StartHeapProfiling(version_info::Channel::UNKNOWN, - ProfilerProcessType::kBrowser, profiling_enabled, - callbacks.first_snapshot_callback(), + StartHeapProfiling(version_info::Channel::UNKNOWN, ProcessType::kBrowser, + profiling_enabled, callbacks.first_snapshot_callback(), callbacks.collector_callback()); histogram_tester_.ExpectUniqueSample( "HeapProfiling.InProcess.Enabled.Browser", profiling_enabled, 1); @@ -1026,12 +1054,22 @@ TEST_P(HeapProfilerControllerChannelTest, UnknownChannel) { constexpr FeatureTestParams kProcessConfigs[] = { // Enabled in parent process only. { + .supported_processes = {ProcessType::kBrowser}, .stable = {.expect_browser_sample = true, .expect_child_sample = false}, }, + // Enabled in child process only. + // Central control only samples child processes when the browser process is + // sampled, so no samples are expected even though sampling is supported in + // the child process. + { + .supported_processes = {ProcessType::kUtility}, + .stable = {.expect_browser_sample = false, + .expect_child_sample = false}, + }, // Enabled in parent and child processes. { + .supported_processes = {ProcessType::kBrowser, ProcessType::kUtility}, .stable = {.expect_browser_sample = true, .expect_child_sample = true}, - .utility_snapshot_prob = 100, }, }; @@ -1042,10 +1080,11 @@ INSTANTIATE_TEST_SUITE_P(All, ::testing::ValuesIn(kProcessConfigs)); TEST_P(HeapProfilerControllerProcessTest, BrowserProcess) { - // This test always enables profiling in the browser process. (Disabled is - // tested in HeapProfilerControllerChannelTest.) + const bool profiling_enabled = + base::Contains(GetParam().supported_processes, ProcessType::kBrowser); ScopedCallbacks callbacks = CreateScopedCallbacks( - /*expect_take_snapshot=*/true, GetParam().stable.expect_browser_sample, + /*expect_take_snapshot=*/profiling_enabled, + GetParam().stable.expect_browser_sample, /*use_other_process_callback=*/GetParam().stable.expect_child_sample); // Mock the child end of the SnapshotController mojo pipe. (Only used when @@ -1054,27 +1093,31 @@ TEST_P(HeapProfilerControllerProcessTest, BrowserProcess) { mojo::Receiver<mojom::SnapshotController> mock_receiver( &mock_child_snapshot_controller); - StartHeapProfiling( - version_info::Channel::STABLE, ProfilerProcessType::kBrowser, - /*expect_enabled=*/true, callbacks.first_snapshot_callback(), - callbacks.collector_callback()); + StartHeapProfiling(version_info::Channel::STABLE, ProcessType::kBrowser, + profiling_enabled, callbacks.first_snapshot_callback(), + callbacks.collector_callback()); histogram_tester_.ExpectUniqueSample( - "HeapProfiling.InProcess.Enabled.Browser", true, 1); - histogram_tester_.ExpectUniqueSample("HeapProfiling.InProcess.Enabled", true, - 1); + "HeapProfiling.InProcess.Enabled.Browser", profiling_enabled, 1); + histogram_tester_.ExpectUniqueSample("HeapProfiling.InProcess.Enabled", + profiling_enabled, 1); constexpr int kTestChildProcessId = 1; - ASSERT_TRUE(controller_->GetBrowserProcessSnapshotController()); + if (profiling_enabled) { + ASSERT_TRUE(controller_->GetBrowserProcessSnapshotController()); - // This callback should be invoked from AppendCommandLineSwitchForChildProcess - // to bind the child end of the mojo pipe. - controller_->GetBrowserProcessSnapshotController() - ->SetBindRemoteForChildProcessCallback(base::BindLambdaForTesting( - [&](int child_process_id, - mojo::PendingReceiver<mojom::SnapshotController> receiver) { - EXPECT_EQ(child_process_id, kTestChildProcessId); - mock_receiver.Bind(std::move(receiver)); - })); + // This callback should be invoked from + // AppendCommandLineSwitchForChildProcess to bind the child end of the mojo + // pipe. + controller_->GetBrowserProcessSnapshotController() + ->SetBindRemoteForChildProcessCallback(base::BindLambdaForTesting( + [&](int child_process_id, + mojo::PendingReceiver<mojom::SnapshotController> receiver) { + EXPECT_EQ(child_process_id, kTestChildProcessId); + mock_receiver.Bind(std::move(receiver)); + })); + } else { + EXPECT_FALSE(controller_->GetBrowserProcessSnapshotController()); + } // Simulate a child process launch. If profiling is enabled in both browser // and child processes, this will bind the browser end of the mojo pipe to the @@ -1082,7 +1125,7 @@ TEST_P(HeapProfilerControllerProcessTest, BrowserProcess) { // child end to `mock_child_snapshot_controller`. base::CommandLine child_command_line(base::CommandLine::NO_PROGRAM); controller_->AppendCommandLineSwitchForChildProcess( - &child_command_line, ProfilerProcessType::kUtility, kTestChildProcessId); + &child_command_line, ProcessType::kUtility, kTestChildProcessId); if (GetParam().stable.expect_child_sample) { EXPECT_CALL(mock_child_snapshot_controller, TakeSnapshot(100, 0)) @@ -1100,12 +1143,12 @@ TEST_P(HeapProfilerControllerProcessTest, BrowserProcess) { } TEST_P(HeapProfilerControllerProcessTest, ChildProcess) { - // TakeSnapshot() is called in the child process when the browser process - // triggers it, which always happens in this test when `utility_snapshot_prob` - // > 0. - const bool profiling_enabled = GetParam().utility_snapshot_prob > 0; + const bool profiling_enabled = + base::Contains(GetParam().supported_processes, ProcessType::kUtility); + // TakeSnapshot() is only called in the child process when the browser process + // triggers it. ScopedCallbacks callbacks = CreateScopedCallbacks( - /*expect_take_snapshot=*/profiling_enabled, + /*expect_take_snapshot=*/GetParam().stable.expect_child_sample, /*expect_sampled_profile=*/GetParam().stable.expect_child_sample, /*use_other_process_callback=*/true); @@ -1130,22 +1173,25 @@ TEST_P(HeapProfilerControllerProcessTest, ChildProcess) { base::test::ScopedCommandLine scoped_command_line; HeapProfilerController::AppendCommandLineSwitchForTesting( - scoped_command_line.GetProcessCommandLine(), - ProfilerProcessType::kUtility, kTestChildProcessId, - fake_browser_snapshot_controller.get()); + scoped_command_line.GetProcessCommandLine(), ProcessType::kUtility, + kTestChildProcessId, fake_browser_snapshot_controller.get()); - // Simulate the browser process taking a sample after a delay. + // Simulate the browser process taking a sample after a delay. If profiling + // isn't enabled in the browser process, just quit waiting after the delay. + base::OnceClosure browser_snapshot_callback = base::DoNothing(); + if (base::Contains(GetParam().supported_processes, ProcessType::kBrowser)) { + browser_snapshot_callback = base::BindOnce( + &BrowserProcessSnapshotController::TakeSnapshotsOnSnapshotSequence, + std::move(fake_browser_snapshot_controller)); + } snapshot_task_runner->PostDelayedTask( FROM_HERE, - base::BindOnce( - &BrowserProcessSnapshotController::TakeSnapshotsOnSnapshotSequence, - std::move(fake_browser_snapshot_controller)) + std::move(browser_snapshot_callback) .Then(callbacks.other_process_callback()), TestTimeouts::action_timeout()); - StartHeapProfiling(version_info::Channel::STABLE, - ProfilerProcessType::kUtility, profiling_enabled, - callbacks.first_snapshot_callback(), + StartHeapProfiling(version_info::Channel::STABLE, ProcessType::kUtility, + profiling_enabled, callbacks.first_snapshot_callback(), callbacks.collector_callback()); histogram_tester_.ExpectUniqueSample( "HeapProfiling.InProcess.Enabled.Utility", profiling_enabled, 1); @@ -1189,8 +1235,8 @@ auto GetProfileMetadataFunc(std::string_view name) { // End-to-end test with multiple child processes. constexpr FeatureTestParams kMultipleChildConfigs[] = { { - .gpu_snapshot_prob = 100, - .network_snapshot_prob = 100, + .supported_processes = {ProcessType::kBrowser, ProcessType::kGpu, + ProcessType::kUtility, ProcessType::kRenderer}, .renderer_snapshot_prob = 66, .utility_snapshot_prob = 50, }, @@ -1217,19 +1263,19 @@ TEST_P(HeapProfilerControllerMultipleChildTest, EndToEnd) { // Process types to test. Each will make a different // number of memory allocations so their reports are all different. - const std::vector<std::pair<ProfilerProcessType, size_t>> kProcessesToTest{ - {ProfilerProcessType::kBrowser, 0}, - {ProfilerProcessType::kGpu, 1}, + const std::vector<std::pair<ProcessType, size_t>> kProcessesToTest{ + {ProcessType::kBrowser, 0}, + {ProcessType::kGpu, 1}, // 2 utility processes. - {ProfilerProcessType::kUtility, 2}, - {ProfilerProcessType::kUtility, 3}, + {ProcessType::kUtility, 2}, + {ProcessType::kUtility, 3}, // 5 renderer processes including one with no samples. The first one will // be ignored to simulate the embedder refusing to profile it. - {ProfilerProcessType::kRenderer, 10}, - {ProfilerProcessType::kRenderer, 0}, - {ProfilerProcessType::kRenderer, 4}, - {ProfilerProcessType::kRenderer, 5}, - {ProfilerProcessType::kRenderer, 6}, + {ProcessType::kRenderer, 10}, + {ProcessType::kRenderer, 0}, + {ProcessType::kRenderer, 4}, + {ProcessType::kRenderer, 5}, + {ProcessType::kRenderer, 6}, }; // Expect only 1 utility process and 3 renderer processes to be sampled due @@ -1257,10 +1303,10 @@ TEST_P(HeapProfilerControllerMultipleChildTest, EndToEnd) { task_runner->DeleteSoon(FROM_HERE, controller_.release()); })); - CreateHeapProfiler( - version_info::Channel::STABLE, ProfilerProcessType::kBrowser, - /*expect_enabled=*/true, std::move(stop_after_first_snapshot_callback), - callbacks.collector_callback()); + CreateHeapProfiler(version_info::Channel::STABLE, ProcessType::kBrowser, + /*expect_enabled=*/true, + std::move(stop_after_first_snapshot_callback), + callbacks.collector_callback()); ASSERT_TRUE(controller_); // Start all processes in `kProcessesToTest` except the browser. @@ -1286,11 +1332,10 @@ TEST_P(HeapProfilerControllerMultipleChildTest, EndToEnd) { bool renderer_was_skipped = false; for (const auto [process_type, num_allocations] : kProcessesToTest) { - if (process_type != ProfilerProcessType::kBrowser) { + if (process_type != ProcessType::kBrowser) { // Skip the first renderer. bool should_profile = true; - if (process_type == ProfilerProcessType::kRenderer && - !renderer_was_skipped) { + if (process_type == ProcessType::kRenderer && !renderer_was_skipped) { should_profile = false; renderer_was_skipped = true; } diff --git a/components/heap_profiling/in_process/heap_profiler_parameters.cc b/components/heap_profiling/in_process/heap_profiler_parameters.cc index a7c84b5b4a976..10a738c3ac27e 100644 --- a/components/heap_profiling/in_process/heap_profiler_parameters.cc +++ b/components/heap_profiling/in_process/heap_profiler_parameters.cc @@ -4,14 +4,19 @@ #include "components/heap_profiling/in_process/heap_profiler_parameters.h" -#include "base/check_op.h" +#include <string> +#include <string_view> + +#include "base/command_line.h" #include "base/feature_list.h" +#include "base/json/json_reader.h" +#include "base/json/json_value_converter.h" #include "base/metrics/field_trial_params.h" -#include "base/notreached.h" -#include "base/numerics/safe_conversions.h" #include "base/time/time.h" +#include "base/values.h" #include "build/build_config.h" #include "components/sampling_profiler/process_type.h" +#include "components/variations/variations_switches.h" namespace heap_profiling { @@ -49,60 +54,65 @@ constexpr double kDefaultStableProbability = 0.01; // provider if it's on a non-stable channel. constexpr double kDefaultNonStableProbability = 0.5; -// The probability of including a child process in each snapshot that's taken, -// as a percentage from 0 to 100. Defaults to 100, but can be set lower to -// sub-sample process types that are very common (mainly renderers) to keep data -// volume low. Samples from child processes are weighted in inverse proportion -// to the snapshot probability to normalize the aggregated results. Set to 0 to -// disable sampling a process completely. +constexpr HeapProfilerParameters kDefaultHeapProfilerParameters{ + .is_supported = false, + // If a process overrides `is_supported`, use the following defaults. + .stable_probability = kDefaultStableProbability, + .nonstable_probability = kDefaultNonStableProbability, + .sampling_rate_bytes = kDefaultSamplingRateBytes, + .collection_interval = kDefaultCollectionInterval, +}; -constexpr base::FeatureParam<int> kGpuSnapshotProbability{ - &kHeapProfilerReporting, "gpu-prob-pct", 100}; +// Feature parameters. -constexpr base::FeatureParam<int> kNetworkSnapshotProbability{ - &kHeapProfilerReporting, "network-prob-pct", 100}; +// JSON-encoded parameter map that will set the default parameters for the +// heap profiler unless overridden by the process-specific parameters below. +constexpr base::FeatureParam<std::string> kDefaultParameters{ + &kHeapProfilerReporting, "default-params", ""}; -// Sample 10% of renderer processes by default, because last time this was -// evaluated (2024-08) the 50th %ile of renderer process count -// (Memory.RenderProcessHost.Count.All) ranged from 8 on Windows to 18 on Mac. -// 10% is an easy default between 1/18 and 1/8. -constexpr base::FeatureParam<int> kRendererSnapshotProbability{ - &kHeapProfilerReporting, "renderer-prob-pct", 10}; +// JSON-encoded parameter map that will override the default parameters for the +// browser process. +constexpr base::FeatureParam<std::string> kBrowserProcessParameters{ + &kHeapProfilerReporting, "browser-process-params", ""}; -// Sample 50% of utility processes by default, because last time this was -// evaluated (2024-08) the profiler collected 1.8x as many snapshots on Mac and -// 2.4x as many snapshots on Windows for each browser process snapshot. -constexpr base::FeatureParam<int> kUtilitySnapshotProbability{ - &kHeapProfilerReporting, "utility-prob-pct", 50}; +// JSON-encoded parameter map that will override the default parameters for +// renderer processes. +constexpr base::FeatureParam<std::string> kRendererProcessParameters{ + &kHeapProfilerReporting, "renderer-process-params", ""}; -// The sampling rates of each process type, in bytes. +// JSON-encoded parameter map that will override the default parameters for the +// GPU process. +constexpr base::FeatureParam<std::string> kGPUProcessParameters{ + &kHeapProfilerReporting, "gpu-process-params", ""}; -constexpr base::FeatureParam<int> kBrowserSamplingRateBytes{ - &kHeapProfilerReporting, "browser-sampling-rate-bytes", - kDefaultSamplingRateBytes}; +// JSON-encoded parameter map that will override the default parameters for +// utility processes. +constexpr base::FeatureParam<std::string> kUtilityProcessParameters{ + &kHeapProfilerReporting, "utility-process-params", ""}; -// Use half the threshold used in the browser process, because last time it was -// validated the GPU process allocated a bit over half as much memory at the -// median. -constexpr base::FeatureParam<int> kGpuSamplingRateBytes{ - &kHeapProfilerReporting, "gpu-sampling-rate-bytes", - kDefaultSamplingRateBytes / 2}; +// JSON-encoded parameter map that will override the default parameters for the +// network process. +constexpr base::FeatureParam<std::string> kNetworkProcessParameters{ + &kHeapProfilerReporting, "network-process-params", ""}; -constexpr base::FeatureParam<int> kNetworkSamplingRateBytes{ - &kHeapProfilerReporting, "network-sampling-rate-bytes", - kDefaultSamplingRateBytes}; - -constexpr base::FeatureParam<int> kRendererSamplingRateBytes{ - &kHeapProfilerReporting, "renderer-sampling-rate-bytes", - kDefaultSamplingRateBytes}; - -// Use 1/10th the threshold used in the browser process, because last time it -// was validated with the default sampling rate (2024-08) the sampler collected -// 6% to 11% as many samples per snapshot in the utility process, depending on -// platform. -constexpr base::FeatureParam<int> kUtilitySamplingRateBytes{ - &kHeapProfilerReporting, "utility-sampling-rate-bytes", - kDefaultSamplingRateBytes / 10}; +// Interprets `value` as a positive number of minutes, and writes the converted +// value to `result`. If `value` contains anything other than a positive +// integer, returns false to indicate a conversion failure. +bool ConvertCollectionInterval(const base::Value* value, + base::TimeDelta* result) { + if (!value) { + // Missing values are ok, so report success without updating `result`. + return true; + } + if (value->is_int()) { + const int minutes = value->GetInt(); + if (minutes > 0) { + *result = base::Minutes(minutes); + return true; + } + } + return false; +} } // namespace @@ -110,65 +120,141 @@ BASE_FEATURE(kHeapProfilerReporting, "HeapProfilerReporting", base::FEATURE_ENABLED_BY_DEFAULT); -const base::FeatureParam<double> kStableProbability{ - &kHeapProfilerReporting, "stable-probability", kDefaultStableProbability}; +// TODO(crbug.com/40840943): The process sampling probabilities are separate +// FeatureParams, but other per-process parameters are parsed from a JSON string +// in a single FeatureParam. The JSON parameters are too complicated: split them +// up into separate FeatureParams. -const base::FeatureParam<double> kNonStableProbability{ - &kHeapProfilerReporting, "nonstable-probability", - kDefaultNonStableProbability}; +const base::FeatureParam<int> kGpuSnapshotProbability{&kHeapProfilerReporting, + "gpu-prob-pct", 100}; -const base::FeatureParam<base::TimeDelta> kCollectionInterval{ - &kHeapProfilerReporting, "collection-interval", kDefaultCollectionInterval}; +const base::FeatureParam<int> kNetworkSnapshotProbability{ + &kHeapProfilerReporting, "network-prob-pct", 100}; -size_t GetSamplingRateForProcess( - sampling_profiler::ProfilerProcessType process_type) { - int sampling_rate_bytes; - switch (process_type) { - case sampling_profiler::ProfilerProcessType::kBrowser: - sampling_rate_bytes = kBrowserSamplingRateBytes.Get(); - break; - case sampling_profiler::ProfilerProcessType::kRenderer: - sampling_rate_bytes = kRendererSamplingRateBytes.Get(); - break; - case sampling_profiler::ProfilerProcessType::kGpu: - sampling_rate_bytes = kGpuSamplingRateBytes.Get(); - break; - case sampling_profiler::ProfilerProcessType::kUtility: - sampling_rate_bytes = kUtilitySamplingRateBytes.Get(); - break; - case sampling_profiler::ProfilerProcessType::kNetworkService: - sampling_rate_bytes = kNetworkSamplingRateBytes.Get(); - break; - case sampling_profiler::ProfilerProcessType::kUnknown: - default: - // Profiler should not be enabled for these process types. - NOTREACHED(); - } - return base::saturated_cast<size_t>(sampling_rate_bytes); +// Sample 10% of renderer processes by default, because last time this was +// evaluated (2024-08) the 50th %ile of renderer process count +// (Memory.RenderProcessHost.Count.All) ranged from 8 on Windows to 18 on Mac. +// 10% is an easy default between 1/18 and 1/8. +const base::FeatureParam<int> kRendererSnapshotProbability{ + &kHeapProfilerReporting, "renderer-prob-pct", 10}; + +// Sample 50% of utility processes by default, because last time this was +// evaluated (2024-08) the profiler collected 1.8x as many snapshots on Mac and +// 2.4x as many snapshots on Windows for each browser process snapshot. +const base::FeatureParam<int> kUtilitySnapshotProbability{ + &kHeapProfilerReporting, "utility-prob-pct", 50}; + +// static +void HeapProfilerParameters::RegisterJSONConverter( + base::JSONValueConverter<HeapProfilerParameters>* converter) { + converter->RegisterBoolField("is-supported", + &HeapProfilerParameters::is_supported); + converter->RegisterDoubleField("stable-probability", + &HeapProfilerParameters::stable_probability); + converter->RegisterDoubleField( + "nonstable-probability", &HeapProfilerParameters::nonstable_probability); + converter->RegisterIntField("sampling-rate-bytes", + &HeapProfilerParameters::sampling_rate_bytes); + converter->RegisterCustomValueField( + "collection-interval-minutes", + &HeapProfilerParameters::collection_interval, &ConvertCollectionInterval); } -int GetSnapshotProbabilityForProcess( +bool HeapProfilerParameters::UpdateFromJSON(std::string_view json_string) { + if (json_string.empty()) + return true; + + base::JSONValueConverter<HeapProfilerParameters> converter; + std::optional<base::Value> value = + base::JSONReader::Read(json_string, base::JSON_ALLOW_TRAILING_COMMAS | + base::JSON_ALLOW_COMMENTS); + if (value && converter.Convert(*value, this)) + return true; + + // Error reading JSON params. Disable the heap sampler. This will be reported + // when HeapProfilerController logs HeapProfiling.InProcess.Enabled. + is_supported = false; + return false; +} + +HeapProfilerParameters GetDefaultHeapProfilerParameters() { + HeapProfilerParameters params = kDefaultHeapProfilerParameters; + params.UpdateFromJSON(kDefaultParameters.Get()); + return params; +} + +HeapProfilerParameters GetHeapProfilerParametersForProcess( sampling_profiler::ProfilerProcessType process_type) { - int snapshot_probability_pct; + using Process = sampling_profiler::ProfilerProcessType; + + HeapProfilerParameters params = kDefaultHeapProfilerParameters; + + // Apply per-process defaults. switch (process_type) { - case sampling_profiler::ProfilerProcessType::kGpu: - snapshot_probability_pct = kGpuSnapshotProbability.Get(); + case Process::kBrowser: + params.is_supported = true; break; - case sampling_profiler::ProfilerProcessType::kNetworkService: - snapshot_probability_pct = kNetworkSnapshotProbability.Get(); + case Process::kNetworkService: + params.is_supported = true; break; - case sampling_profiler::ProfilerProcessType::kRenderer: - snapshot_probability_pct = kRendererSnapshotProbability.Get(); + case Process::kGpu: + params.is_supported = true; + // Use half the threshold used in the browser process, because last time + // it was validated the GPU process allocated a bit over half as much + // memory at the median. + params.sampling_rate_bytes = params.sampling_rate_bytes / 2; break; - case sampling_profiler::ProfilerProcessType::kUtility: - snapshot_probability_pct = kUtilitySnapshotProbability.Get(); + case Process::kRenderer: + params.is_supported = true; break; + case Process::kUtility: + params.is_supported = true; + // Use 1/10th the threshold used in the browser process, because last time + // it was validated with the default sampling rate (2024-08) the sampler + // collected 6% to 11% as many samples per snapshot in the utility + // process, depending on platform. + params.sampling_rate_bytes = params.sampling_rate_bytes / 10; + break; + case Process::kUnknown: default: - NOTREACHED(); + // Do nothing. Profiler hasn't been tested in these process types. + break; } - CHECK_GE(snapshot_probability_pct, 0); - CHECK_LE(snapshot_probability_pct, 100); - return snapshot_probability_pct; + + if (base::CommandLine::ForCurrentProcess()->HasSwitch( + variations::switches::kEnableBenchmarking) || + !base::FeatureList::IsEnabled(kHeapProfilerReporting)) { + params.is_supported = false; + return params; + } + + // Override with field trial parameters if any are set. + if (!params.UpdateFromJSON(kDefaultParameters.Get())) { + // After an error is detected don't alter `params` further. + return params; + } + switch (process_type) { + case Process::kBrowser: + params.UpdateFromJSON(kBrowserProcessParameters.Get()); + break; + case Process::kRenderer: + params.UpdateFromJSON(kRendererProcessParameters.Get()); + break; + case Process::kGpu: + params.UpdateFromJSON(kGPUProcessParameters.Get()); + break; + case Process::kUtility: + params.UpdateFromJSON(kUtilityProcessParameters.Get()); + break; + case Process::kNetworkService: + params.UpdateFromJSON(kNetworkProcessParameters.Get()); + break; + case Process::kUnknown: + default: + // Do nothing. Profiler hasn't been tested in these process types. + break; + } + return params; } } // namespace heap_profiling diff --git a/components/heap_profiling/in_process/heap_profiler_parameters.h b/components/heap_profiling/in_process/heap_profiler_parameters.h index b5b81cd0b824b..9d9a1517debda 100644 --- a/components/heap_profiling/in_process/heap_profiler_parameters.h +++ b/components/heap_profiling/in_process/heap_profiler_parameters.h @@ -5,7 +5,10 @@ #ifndef COMPONENTS_HEAP_PROFILING_IN_PROCESS_HEAP_PROFILER_PARAMETERS_H_ #define COMPONENTS_HEAP_PROFILING_IN_PROCESS_HEAP_PROFILER_PARAMETERS_H_ +#include <string_view> + #include "base/feature_list.h" +#include "base/json/json_value_converter.h" #include "base/metrics/field_trial_params.h" #include "base/time/time.h" #include "components/sampling_profiler/process_type.h" @@ -20,24 +23,54 @@ namespace heap_profiling { // reporting is enabled. BASE_DECLARE_FEATURE(kHeapProfilerReporting); -// Chance that this client will report heap samples through a metrics -// provider if it's on the stable channel. -extern const base::FeatureParam<double> kStableProbability; +// The probability of including a child process in each snapshot that's taken, +// as a percentage from 0 to 100. Defaults to 100, but can be set lower to +// sub-sample process types that are very common (mainly renderers) to keep data +// volume low. Samples from child processes are weighted in inverse proportion +// to the snapshot probability to normalize the aggregated results. +extern const base::FeatureParam<int> kGpuSnapshotProbability; +extern const base::FeatureParam<int> kNetworkSnapshotProbability; +extern const base::FeatureParam<int> kRendererSnapshotProbability; +extern const base::FeatureParam<int> kUtilitySnapshotProbability; -// Chance that this client will report heap samples through a metrics -// provider if it's on a non-stable channel. -extern const base::FeatureParam<double> kNonStableProbability; +// Parameters to control the heap profiler. +struct HeapProfilerParameters { + // True if heap profiling is supported, false otherwise. + bool is_supported = false; -// Mean time between snapshots. -extern const base::FeatureParam<base::TimeDelta> kCollectionInterval; + // Chance that this client will report heap samples through a metrics + // provider if it's on the stable channel. + double stable_probability = 0.0; -// Returns the sampling rate in bytes to use for `process_type`. -size_t GetSamplingRateForProcess( - sampling_profiler::ProfilerProcessType process_type); + // Chance that this client will report heap samples through a metrics + // provider if it's on a non-stable channel. + double nonstable_probability = 0.0; -// Returns the probability of sampling a `process_type` process in each -// snapshot, from 0 to 100. -int GetSnapshotProbabilityForProcess( + // Mean heap sampling interval in bytes. + int sampling_rate_bytes = 0; + + // Mean time between snapshots. + base::TimeDelta collection_interval; + + // Invoked by JSONValueConverter to parse parameters from JSON. + static void RegisterJSONConverter( + base::JSONValueConverter<HeapProfilerParameters>* converter); + + // Overwrites this object's fields with parameters parsed from `json_string`. + // Missing parameters will not be touched. If parsing fails, returns false and + // sets `is_supported` to false to ensure heap profiling doesn't run with + // invalid parameters. + bool UpdateFromJSON(std::string_view json_string); +}; + +// Returns a default set of parameters to use if not overridden for a +// specific process. +HeapProfilerParameters GetDefaultHeapProfilerParameters(); + +// Returns the set of process parameters to use for `process_type`. This will be +// identical to the result of GetDefaultHeapProfilerParameters() unless +// overridden by a field trial. +HeapProfilerParameters GetHeapProfilerParametersForProcess( sampling_profiler::ProfilerProcessType process_type); } // namespace heap_profiling diff --git a/components/heap_profiling/in_process/heap_profiler_parameters_unittest.cc b/components/heap_profiling/in_process/heap_profiler_parameters_unittest.cc new file mode 100644 index 0000000000000..97430a27aa27f --- /dev/null +++ b/components/heap_profiling/in_process/heap_profiler_parameters_unittest.cc @@ -0,0 +1,233 @@ +// Copyright 2022 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "components/heap_profiling/in_process/heap_profiler_parameters.h" + +#include "base/command_line.h" +#include "base/test/scoped_feature_list.h" +#include "base/time/time.h" +#include "components/sampling_profiler/process_type.h" +#include "components/variations/variations_switches.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace heap_profiling { + +namespace { + +using ::testing::AllOf; +using ::testing::Field; + +// Can't define operator== because gmock has a conflicting operator== in an +// internal namespace. +auto MatchesParameters(const HeapProfilerParameters& expected) { + return AllOf( + Field("is_supported", &HeapProfilerParameters::is_supported, + expected.is_supported), + Field("stable_probability", &HeapProfilerParameters::stable_probability, + expected.stable_probability), + Field("nonstable_probability", + &HeapProfilerParameters::nonstable_probability, + expected.nonstable_probability), + Field("sampling_rate_bytes", &HeapProfilerParameters::sampling_rate_bytes, + expected.sampling_rate_bytes), + Field("collection_interval", &HeapProfilerParameters::collection_interval, + expected.collection_interval)); +} + +TEST(HeapProfilerParametersTest, ParseEmptyParameters) { + constexpr char kJSONParams[] = R"({})"; + HeapProfilerParameters params; + EXPECT_TRUE(params.UpdateFromJSON(kJSONParams)); + EXPECT_THAT(params, MatchesParameters({})); +} + +TEST(HeapProfilerParametersTest, ParseParameters) { + constexpr char kJSONParams[] = R"({ + "is-supported": true, + "stable-probability": 0.1, + // Comments should be allowed. + // Double parameters should convert from integers. + "nonstable-probability": 1, + "sampling-rate-bytes": 1000, + "collection-interval-minutes": 30, + })"; + HeapProfilerParameters params; + EXPECT_TRUE(params.UpdateFromJSON(kJSONParams)); + EXPECT_THAT(params, MatchesParameters({ + .is_supported = true, + .stable_probability = 0.1, + .nonstable_probability = 1.0, + .sampling_rate_bytes = 1000, + .collection_interval = base::Minutes(30), + })); +} + +TEST(HeapProfilerParametersTest, ParsePartialParameters) { + constexpr char kJSONParams[] = R"({ + "is-supported": false, + "stable-probability": 0.5, + "collection-interval-minutes": 60, + })"; + // Only the parameters that are included in the JSON should be overwritten. + HeapProfilerParameters params{ + .is_supported = true, + .stable_probability = 0.1, + .nonstable_probability = 0.2, + .sampling_rate_bytes = 1000, + .collection_interval = base::Minutes(30), + }; + EXPECT_TRUE(params.UpdateFromJSON(kJSONParams)); + EXPECT_THAT(params, MatchesParameters({ + .is_supported = false, + .stable_probability = 0.5, + .nonstable_probability = 0.2, + .sampling_rate_bytes = 1000, + .collection_interval = base::Minutes(60), + })); +} + +TEST(HeapProfilerParametersTest, ParseInvalidParameters) { + constexpr char kJSONParams[] = R"({ + "collection-interval-minutes": -1, + })"; + HeapProfilerParameters params; + EXPECT_FALSE(params.UpdateFromJSON(kJSONParams)); + EXPECT_FALSE(params.is_supported); +} + +// Test that heap profiling is not supported for any process type when +// --enable-benchmarking is specified on the command line. +TEST(HeapProfilerParametersTest, EnableBenchmarking) { + base::CommandLine::ForCurrentProcess()->AppendSwitch( + variations::switches::kEnableBenchmarking); + + using Process = sampling_profiler::ProfilerProcessType; + EXPECT_FALSE(GetDefaultHeapProfilerParameters().is_supported); + EXPECT_FALSE( + GetHeapProfilerParametersForProcess(Process::kBrowser).is_supported); + EXPECT_FALSE(GetHeapProfilerParametersForProcess(Process::kGpu).is_supported); + EXPECT_FALSE( + GetHeapProfilerParametersForProcess(Process::kRenderer).is_supported); + EXPECT_FALSE( + GetHeapProfilerParametersForProcess(Process::kUtility).is_supported); + EXPECT_FALSE(GetHeapProfilerParametersForProcess(Process::kNetworkService) + .is_supported); +} + +TEST(HeapProfilerParametersTest, ApplyParameters) { + constexpr char kDefaultParams[] = R"({ + "is-supported": false, + "stable-probability": 0.1, + "nonstable-probability": 0.2, + "sampling-rate-bytes": 1000, + "collection-interval-minutes": 15, + })"; + constexpr char kBrowserParams[] = R"({ + "sampling-rate-bytes": 1001, + })"; + constexpr char kGPUParams[] = R"({ + "is-supported": true, + "sampling-rate-bytes": 1002, + "collection-interval-minutes": 60, + })"; + constexpr char kRendererParams[] = R"({ + "is-supported": false, + "sampling-rate-bytes": 1003, + })"; + + base::test::ScopedFeatureList feature_list; + feature_list.InitAndEnableFeatureWithParameters( + kHeapProfilerReporting, { + {"default-params", kDefaultParams}, + {"browser-process-params", kBrowserParams}, + {"gpu-process-params", kGPUParams}, + {"renderer-process-params", kRendererParams}, + {"utility-process-params", "{}"}, + }); + + EXPECT_THAT(GetDefaultHeapProfilerParameters(), + MatchesParameters({ + .is_supported = false, + .stable_probability = 0.1, + .nonstable_probability = 0.2, + .sampling_rate_bytes = 1000, + .collection_interval = base::Minutes(15), + })); + + using Process = sampling_profiler::ProfilerProcessType; + EXPECT_THAT(GetHeapProfilerParametersForProcess(Process::kBrowser), + MatchesParameters({ + .is_supported = false, + .stable_probability = 0.1, + .nonstable_probability = 0.2, + .sampling_rate_bytes = 1001, + .collection_interval = base::Minutes(15), + })); + + EXPECT_THAT(GetHeapProfilerParametersForProcess(Process::kGpu), + MatchesParameters({ + .is_supported = true, + .stable_probability = 0.1, + .nonstable_probability = 0.2, + .sampling_rate_bytes = 1002, + .collection_interval = base::Minutes(60), + })); + + EXPECT_THAT(GetHeapProfilerParametersForProcess(Process::kRenderer), + MatchesParameters({ + .is_supported = false, + .stable_probability = 0.1, + .nonstable_probability = 0.2, + .sampling_rate_bytes = 1003, + .collection_interval = base::Minutes(15), + })); + + EXPECT_THAT(GetHeapProfilerParametersForProcess(Process::kUtility), + MatchesParameters({ + .is_supported = false, + .stable_probability = 0.1, + .nonstable_probability = 0.2, + .sampling_rate_bytes = 1000, + .collection_interval = base::Minutes(15), + })); + + EXPECT_THAT(GetHeapProfilerParametersForProcess(Process::kNetworkService), + MatchesParameters({ + .is_supported = false, + .stable_probability = 0.1, + .nonstable_probability = 0.2, + .sampling_rate_bytes = 1000, + .collection_interval = base::Minutes(15), + })); +} + +TEST(HeapProfilerParametersTest, ApplyInvalidParameters) { + constexpr char kDefaultParams[] = R"({ + "is-supported": true, + "collection-interval-minutes": "unexpected string", + })"; + // Ensure that valid per-process params don't overwrite invalid default + // params. + constexpr char kBrowserParams[] = R"({ + "is-supported": true, + "collection-interval-minutes": 1, + })"; + + base::test::ScopedFeatureList feature_list; + feature_list.InitAndEnableFeatureWithParameters( + kHeapProfilerReporting, { + {"default-params", kDefaultParams}, + {"browser-process-params", kBrowserParams}, + }); + + EXPECT_FALSE(GetDefaultHeapProfilerParameters().is_supported); + EXPECT_FALSE(GetHeapProfilerParametersForProcess( + sampling_profiler::ProfilerProcessType::kBrowser) + .is_supported); +} + +} // namespace + +} // namespace heap_profiling