Commit f12e2d5c authored by Sigurdur Asgeirsson's avatar Sigurdur Asgeirsson Committed by Commit Bot

RC: Ditch RenderProcessProbe repeated measurement code.

Also drop the relevant RC feature flags and UKM measurement code.

Bug: 755840
Change-Id: I055e5e7fa7f316aaaa9d48defa3450148b912700
Reviewed-on: https://chromium-review.googlesource.com/1163729Reviewed-by: default avatarNico Weber <thakis@chromium.org>
Reviewed-by: default avatarChris Hamilton <chrisha@chromium.org>
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580981}
parent 30d8a3b9
......@@ -2084,8 +2084,6 @@ bool ChromeBrowserMainParts::MainMessageLoopRun(int* result_code) {
performance_monitor::PerformanceMonitor::GetInstance()->StartGatherCycle();
resource_coordinator::RenderProcessProbe::GetInstance()->StartGatherCycle();
metrics::MetricsService::SetExecutionPhase(
metrics::ExecutionPhase::MAIN_MESSAGE_LOOP_RUN,
g_browser_process->local_state());
......
......@@ -18,7 +18,6 @@ namespace resource_coordinator {
class LenientMockRenderProcessProbe : public RenderProcessProbe {
public:
MOCK_METHOD0(StartGatherCycle, void());
MOCK_METHOD0(StartSingleGather, void());
};
using MockRenderProcessProbe =
......
......@@ -22,13 +22,6 @@
namespace resource_coordinator {
namespace {
constexpr base::TimeDelta kDefaultMeasurementInterval =
base::TimeDelta::FromMinutes(10);
} // namespace
constexpr base::TimeDelta RenderProcessProbeImpl::kUninitializedCPUTime;
// static
......@@ -42,49 +35,24 @@ bool RenderProcessProbe::IsEnabled() {
// Check that service_manager is active, GRC is enabled,
// and render process CPU profiling is enabled.
return content::ServiceManagerConnection::GetForProcess() != nullptr &&
resource_coordinator::IsResourceCoordinatorEnabled() &&
base::FeatureList::IsEnabled(features::kGRCRenderProcessCPUProfiling);
resource_coordinator::IsResourceCoordinatorEnabled();
}
RenderProcessProbeImpl::RenderProcessInfo::RenderProcessInfo() = default;
RenderProcessProbeImpl::RenderProcessInfo::~RenderProcessInfo() = default;
RenderProcessProbeImpl::RenderProcessProbeImpl()
: interval_(kDefaultMeasurementInterval) {
UpdateWithFieldTrialParams();
}
RenderProcessProbeImpl::RenderProcessProbeImpl() {}
RenderProcessProbeImpl::~RenderProcessProbeImpl() = default;
void RenderProcessProbeImpl::StartGatherCycle() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
// TODO(siggi): It irks me to have this bit of policy embedded here.
// I feel this should be moved to the caller...
if (!RenderProcessProbeImpl::IsEnabled()) {
return;
}
DCHECK(!is_gather_cycle_started_);
is_gather_cycle_started_ = true;
if (!is_gathering_) {
timer_.Start(
FROM_HERE, base::TimeDelta(), this,
&RenderProcessProbeImpl::RegisterAliveRenderProcessesOnUIThread);
}
}
void RenderProcessProbeImpl::StartSingleGather() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (is_gathering_)
return;
// If the gather cycle is started this measurement will go through early,
// and the interval between measurements will be shortened.
timer_.Start(FROM_HERE, base::TimeDelta(), this,
&RenderProcessProbeImpl::RegisterAliveRenderProcessesOnUIThread);
RegisterAliveRenderProcessesOnUIThread();
}
void RenderProcessProbeImpl::RegisterAliveRenderProcessesOnUIThread() {
......@@ -230,28 +198,19 @@ void RenderProcessProbeImpl::ProcessGlobalMemoryDumpAndDispatchOnIOThread(
UMA_HISTOGRAM_TIMES("ResourceCoordinator.Measurement.Duration",
batch->batch_ended_time - batch->batch_started_time);
// TODO(siggi): UMA record measurement time.
bool should_restart = DispatchMetrics(std::move(batch));
content::BrowserThread::PostTask(
content::BrowserThread::UI, FROM_HERE,
base::BindOnce(&RenderProcessProbeImpl::FinishCollectionOnUIThread,
base::Unretained(this), should_restart));
base::Unretained(this), std::move(batch)));
}
void RenderProcessProbeImpl::FinishCollectionOnUIThread(bool restart_cycle) {
void RenderProcessProbeImpl::FinishCollectionOnUIThread(
mojom::ProcessResourceMeasurementBatchPtr batch) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
DCHECK(is_gathering_);
is_gathering_ = false;
if (restart_cycle && is_gather_cycle_started_) {
timer_.Start(
FROM_HERE, interval_, this,
&RenderProcessProbeImpl::RegisterAliveRenderProcessesOnUIThread);
} else {
is_gather_cycle_started_ = false;
}
AfterFinishCollectionOnUIThread();
DispatchMetricsOnUIThread(std::move(batch));
}
void RenderProcessProbeImpl::RegisterRenderProcesses() {
......@@ -302,14 +261,6 @@ base::ProcessId RenderProcessProbeImpl::GetProcessId(
return info.process.Pid();
}
void RenderProcessProbeImpl::UpdateWithFieldTrialParams() {
int64_t interval_ms = GetGRCRenderProcessCPUProfilingIntervalInMs();
if (interval_ms > 0) {
interval_ = base::TimeDelta::FromMilliseconds(interval_ms);
}
}
SystemResourceCoordinator*
RenderProcessProbeImpl::EnsureSystemResourceCoordinator() {
if (!system_resource_coordinator_) {
......@@ -324,16 +275,14 @@ RenderProcessProbeImpl::EnsureSystemResourceCoordinator() {
return system_resource_coordinator_.get();
}
bool RenderProcessProbeImpl::DispatchMetrics(
void RenderProcessProbeImpl::DispatchMetricsOnUIThread(
mojom::ProcessResourceMeasurementBatchPtr batch) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
SystemResourceCoordinator* system_resource_coordinator =
EnsureSystemResourceCoordinator();
if (system_resource_coordinator && !batch->measurements.empty())
system_resource_coordinator->DistributeMeasurementBatch(std::move(batch));
return true;
}
} // namespace resource_coordinator
......@@ -35,20 +35,13 @@ class RenderProcessProbe {
virtual ~RenderProcessProbe() = default;
// Starts the automatic, timed process metrics collection cycle.
// Can only be invoked from the UI thread.
virtual void StartGatherCycle() = 0;
// Starts a single immediate collection cycle, if a cycle is not already
// in progress. If the timed gather cycle is running, this will preempt the
// next cycle and reset the metronome.
// Starts a single collection cycle if a cycle is not already in progress.
// If a cycle is already in progress, this method will simply return.
virtual void StartSingleGather() = 0;
};
class RenderProcessProbeImpl : public RenderProcessProbe {
public:
void StartGatherCycle() override;
void StartSingleGather() override;
protected:
......@@ -91,45 +84,31 @@ class RenderProcessProbeImpl : public RenderProcessProbe {
base::TimeTicks collection_start_time,
bool success,
std::unique_ptr<memory_instrumentation::GlobalMemoryDump> dump);
// (4) Initiate the next render process metrics collection cycle if the
// cycle has been started and |restart_cycle| is true, which consists of a
// delayed call to perform (1) via a timer.
void FinishCollectionOnUIThread(bool restart_cycle);
// (4) Finish the collection cycle on the UI thread.
void FinishCollectionOnUIThread(
mojom::ProcessResourceMeasurementBatchPtr batch);
// Test seams.
virtual void AfterFinishCollectionOnUIThread() {}
virtual void RegisterRenderProcesses();
virtual void StartMemoryMeasurement(base::TimeTicks collection_start_time);
virtual base::ProcessId GetProcessId(int host_id,
const RenderProcessInfo& info);
// Allows FieldTrial parameters to override defaults.
void UpdateWithFieldTrialParams();
SystemResourceCoordinator* EnsureSystemResourceCoordinator();
// Dispatch the collected metrics, return true if the cycle should restart.
// Dispatch the collected metrics.
// Virtual for testing.
virtual bool DispatchMetrics(mojom::ProcessResourceMeasurementBatchPtr batch);
virtual void DispatchMetricsOnUIThread(
mojom::ProcessResourceMeasurementBatchPtr batch);
// A map of currently running render process host IDs to process.
// This map is accessed alternatively from the UI thread and the IO thread,
// but only one of the two at a time.
RenderProcessInfoMap render_process_info_map_;
// Time duration between measurements.
base::TimeDelta interval_;
// Timer to signal the |RenderProcessProbe| instance
// to conduct its measurements as a regular interval;
base::OneShotTimer timer_;
// Number of measurements collected so far.
size_t current_gather_cycle_ = 0u;
// True if StartGatherCycle has been called.
bool is_gather_cycle_started_ = false;
// True while a gathering cycle is underways on a background thread.
bool is_gathering_ = false;
......
......@@ -34,14 +34,10 @@ class TestingRenderProcessProbe : public RenderProcessProbeImpl {
TestingRenderProcessProbe() = default;
~TestingRenderProcessProbe() override = default;
bool DispatchMetrics(
void DispatchMetricsOnUIThread(
mojom::ProcessResourceMeasurementBatchPtr batch) override {
last_measurement_batch_ = std::move(batch);
return false;
}
void AfterFinishCollectionOnUIThread() override {
current_run_loop_->QuitWhenIdle();
}
......@@ -69,7 +65,6 @@ class TestingRenderProcessProbe : public RenderProcessProbeImpl {
}
size_t current_gather_cycle() const { return current_gather_cycle_; }
bool is_gather_cycle_started() const { return is_gather_cycle_started_; }
void WaitForGather() {
base::RunLoop run_loop;
......@@ -80,8 +75,8 @@ class TestingRenderProcessProbe : public RenderProcessProbeImpl {
current_run_loop_ = nullptr;
}
void StartGatherCycleAndWait() {
StartGatherCycle();
void StartSingleGatherAndWait() {
StartSingleGather();
WaitForGather();
}
......@@ -124,9 +119,7 @@ IN_PROC_BROWSER_TEST_F(RenderProcessProbeBrowserTest,
#endif
// Ensure that the |resource_coordinator| service is enabled.
base::test::ScopedFeatureList feature_list;
feature_list.InitWithFeatures({features::kGlobalResourceCoordinator,
features::kGRCRenderProcessCPUProfiling},
{});
feature_list.InitWithFeatures({features::kGlobalResourceCoordinator}, {});
TestingRenderProcessProbe probe;
......@@ -134,7 +127,7 @@ IN_PROC_BROWSER_TEST_F(RenderProcessProbeBrowserTest,
EXPECT_EQ(0u, probe.current_gather_cycle());
// A tab is already open when the test begins.
probe.StartGatherCycleAndWait();
probe.StartSingleGatherAndWait();
EXPECT_EQ(1u, probe.current_gather_cycle());
size_t initial_size = probe.render_process_info_map().size();
EXPECT_LE(1u, initial_size);
......@@ -164,7 +157,7 @@ IN_PROC_BROWSER_TEST_F(RenderProcessProbeBrowserTest,
WindowOpenDisposition::NEW_FOREGROUND_TAB,
ui_test_utils::BROWSER_TEST_WAIT_FOR_TAB |
ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
probe.StartGatherCycleAndWait();
probe.StartSingleGatherAndWait();
EXPECT_EQ(2u, probe.current_gather_cycle());
EXPECT_EQ(initial_size + 1u, probe.render_process_info_map().size());
EXPECT_EQ(initial_size + 1u,
......@@ -181,7 +174,7 @@ IN_PROC_BROWSER_TEST_F(RenderProcessProbeBrowserTest,
}
size_t info_map_size = info_map.size();
probe.StartGatherCycleAndWait();
probe.StartSingleGatherAndWait();
// Verify that CPU usage is monotonically increasing, though the measurement
// granulatity is such on some OSes that a zero difference is almost certain.
for (const auto& measurement : probe.last_measurement_batch()->measurements) {
......@@ -207,53 +200,11 @@ IN_PROC_BROWSER_TEST_F(RenderProcessProbeBrowserTest,
->GetMainFrame()
->GetProcess()
->FastShutdownIfPossible());
probe.StartGatherCycleAndWait();
probe.StartSingleGatherAndWait();
EXPECT_EQ(4u, probe.current_gather_cycle());
EXPECT_EQ(initial_size, probe.render_process_info_map().size());
EXPECT_EQ(initial_size, probe.last_measurement_batch()->measurements.size());
EXPECT_TRUE(probe.AllMeasurementsAreAtCurrentCycle());
}
IN_PROC_BROWSER_TEST_F(RenderProcessProbeBrowserTest, StartSingleGather) {
// Ensure that the |resource_coordinator| service is enabled.
base::test::ScopedFeatureList feature_list;
feature_list.InitWithFeatures({features::kGlobalResourceCoordinator,
features::kGRCRenderProcessCPUProfiling},
{});
TestingRenderProcessProbe probe;
// Test the gather cycle state.
EXPECT_FALSE(probe.is_gather_cycle_started());
probe.StartGatherCycle();
EXPECT_TRUE(probe.is_gather_cycle_started());
probe.WaitForGather();
EXPECT_FALSE(probe.is_gather_cycle_started());
EXPECT_EQ(1u, probe.current_gather_cycle());
// Test a single gather while the gather cycle is disabled.
probe.StartSingleGather();
EXPECT_FALSE(probe.is_gather_cycle_started());
probe.WaitForGather();
EXPECT_FALSE(probe.is_gather_cycle_started());
EXPECT_EQ(2u, probe.current_gather_cycle());
// Test a single gather followed by starting the gather cycle.
probe.StartSingleGather();
EXPECT_FALSE(probe.is_gather_cycle_started());
probe.StartGatherCycle();
EXPECT_TRUE(probe.is_gather_cycle_started());
probe.WaitForGather();
EXPECT_FALSE(probe.is_gather_cycle_started());
EXPECT_EQ(3u, probe.current_gather_cycle());
// And now a single gather after the cycle is started.
probe.StartGatherCycle();
EXPECT_TRUE(probe.is_gather_cycle_started());
probe.StartSingleGather();
probe.WaitForGather();
EXPECT_FALSE(probe.is_gather_cycle_started());
EXPECT_EQ(4u, probe.current_gather_cycle());
}
} // namespace resource_coordinator
......@@ -16,8 +16,6 @@
namespace resource_coordinator {
const size_t kDefaultMaxCPUUsageMeasurements = 30u;
// Audio is considered to have started playing if the page has never
// previously played audio, or has been silent for at least one minute.
const base::TimeDelta kMaxAudioSlientTimeout = base::TimeDelta::FromMinutes(1);
......@@ -54,8 +52,7 @@ size_t GetNumCoresidentTabs(const PageCoordinationUnitImpl* page_cu) {
return coresident_tabs.size() - 1;
}
MetricsCollector::MetricsCollector()
: max_ukm_cpu_usage_measurements_(kDefaultMaxCPUUsageMeasurements) {
MetricsCollector::MetricsCollector() {
UpdateWithFieldTrialParams();
}
......@@ -135,15 +132,7 @@ void MetricsCollector::OnProcessPropertyChanged(
const ProcessCoordinationUnitImpl* process_cu,
const mojom::PropertyType property_type,
int64_t value) {
if (property_type == mojom::PropertyType::kCPUUsage) {
for (auto* page_cu : process_cu->GetAssociatedPageCoordinationUnits()) {
if (IsCollectingCPUUsageForUkm(page_cu->id())) {
RecordCPUUsageForUkm(page_cu->id(), page_cu->GetCPUUsage(),
GetNumCoresidentTabs(page_cu));
}
}
} else if (property_type ==
mojom::PropertyType::kExpectedTaskQueueingDuration) {
if (property_type == mojom::PropertyType::kExpectedTaskQueueingDuration) {
for (auto* page_cu : process_cu->GetAssociatedPageCoordinationUnits()) {
if (IsCollectingExpectedQueueingTimeForUkm(page_cu->id())) {
int64_t expected_queueing_time;
......@@ -213,14 +202,6 @@ bool MetricsCollector::ShouldReportMetrics(
return page_cu->TimeSinceLastNavigation() > kMetricsReportDelayTimeout;
}
bool MetricsCollector::IsCollectingCPUUsageForUkm(
const CoordinationUnitID& page_cu_id) {
const UkmCollectionState& state = ukm_collection_state_map_[page_cu_id];
return state.ukm_source_id != ukm::kInvalidSourceId &&
state.num_cpu_usage_measurements < max_ukm_cpu_usage_measurements_;
}
bool MetricsCollector::IsCollectingExpectedQueueingTimeForUkm(
const CoordinationUnitID& page_cu_id) {
UkmCollectionState& state = ukm_collection_state_map_[page_cu_id];
......@@ -228,18 +209,6 @@ bool MetricsCollector::IsCollectingExpectedQueueingTimeForUkm(
++state.num_unreported_eqt_measurements >= frequency_ukm_eqt_reported_;
}
void MetricsCollector::RecordCPUUsageForUkm(
const CoordinationUnitID& page_cu_id,
double cpu_usage,
size_t num_coresident_tabs) {
UkmCollectionState& state = ukm_collection_state_map_[page_cu_id];
ukm::builders::CPUUsageMeasurement(state.ukm_source_id)
.SetCPUUsage(cpu_usage)
.SetNumberOfCoresidentTabs(num_coresident_tabs)
.Record(coordination_unit_graph().ukm_recorder());
}
void MetricsCollector::RecordExpectedQueueingTimeForUkm(
const CoordinationUnitID& page_cu_id,
int64_t expected_queueing_time) {
......@@ -257,19 +226,10 @@ void MetricsCollector::UpdateUkmSourceIdForPage(
state.ukm_source_id = ukm_source_id;
// Updating the |ukm_source_id| restarts usage collection.
state.num_cpu_usage_measurements = 0u;
state.num_unreported_eqt_measurements = 0u;
}
void MetricsCollector::UpdateWithFieldTrialParams() {
int64_t interval_ms = GetGRCRenderProcessCPUProfilingIntervalInMs();
int64_t duration_ms = GetGRCRenderProcessCPUProfilingDurationInMs();
if (interval_ms > 0 && duration_ms > 0 && duration_ms >= interval_ms) {
max_ukm_cpu_usage_measurements_ =
static_cast<size_t>(duration_ms / interval_ms);
}
frequency_ukm_eqt_reported_ = base::GetFieldTrialParamByFeatureAsInt(
ukm::kUkmFeature, "FrequencyUKMExpectedQueueingTime",
kDefaultFrequencyUkmEQTReported);
......
......@@ -91,18 +91,13 @@ class MetricsCollector : public CoordinationUnitGraphObserver {
};
struct UkmCollectionState {
size_t num_cpu_usage_measurements = 0u;
int num_unreported_eqt_measurements = 0u;
ukm::SourceId ukm_source_id = ukm::kInvalidSourceId;
};
bool ShouldReportMetrics(const PageCoordinationUnitImpl* page_cu);
bool IsCollectingCPUUsageForUkm(const CoordinationUnitID& page_cu_id);
bool IsCollectingExpectedQueueingTimeForUkm(
const CoordinationUnitID& page_cu_id);
void RecordCPUUsageForUkm(const CoordinationUnitID& page_cu_id,
double cpu_usage,
size_t num_coresident_tabs);
void RecordExpectedQueueingTimeForUkm(const CoordinationUnitID& page_cu_id,
int64_t expected_queueing_time);
void UpdateUkmSourceIdForPage(const CoordinationUnitID& page_cu_id,
......@@ -114,7 +109,6 @@ class MetricsCollector : public CoordinationUnitGraphObserver {
// already reported to avoid reporting multiple metrics.
std::map<CoordinationUnitID, MetricsReportRecord> metrics_report_record_map_;
std::map<CoordinationUnitID, UkmCollectionState> ukm_collection_state_map_;
size_t max_ukm_cpu_usage_measurements_ = 0u;
// The number of reports to wait before reporting ExpectedQueueingTime. For
// example, if |frequency_ukm_eqt_reported_| is 2, then the first value is not
// reported, the second one is, the third one isn't, etc.
......
......@@ -11,28 +11,9 @@
namespace {
constexpr char kUkmPageLoadCPUUsageProfilingTrialName[] =
"UkmPageLoadCPUUsageProfiling";
constexpr char kIntervalInMsParameterName[] = "intervalInMs";
constexpr char kDurationInMsParameterName[] = "durationInMs";
constexpr char kMainThreadTaskLoadLowThresholdParameterName[] =
"mainThreadTaskLoadLowThreshold";
int64_t GetIntegerFieldTrialParam(const std::string& trial_name,
const std::string& parameter_name,
int64_t default_val) {
std::string parameter_str =
base::GetFieldTrialParamValue(trial_name, parameter_name);
int64_t parameter_value;
if (parameter_str.empty() ||
!base::StringToInt64(parameter_str, &parameter_value)) {
return default_val;
}
return parameter_value;
}
} // namespace
namespace features {
......@@ -41,10 +22,6 @@ namespace features {
const base::Feature kGlobalResourceCoordinator{
"GlobalResourceCoordinator", base::FEATURE_ENABLED_BY_DEFAULT};
// Enable render process CPU profiling for GRC.
const base::Feature kGRCRenderProcessCPUProfiling{
"GRCRenderProcessCPUProfiling", base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature kPageAlmostIdle{"PageAlmostIdle",
base::FEATURE_DISABLED_BY_DEFAULT};
......@@ -56,20 +33,6 @@ bool IsResourceCoordinatorEnabled() {
return base::FeatureList::IsEnabled(features::kGlobalResourceCoordinator);
}
bool IsGRCRenderProcessCPUProfilingEnabled() {
return base::FeatureList::IsEnabled(features::kGRCRenderProcessCPUProfiling);
}
int64_t GetGRCRenderProcessCPUProfilingDurationInMs() {
return GetIntegerFieldTrialParam(kUkmPageLoadCPUUsageProfilingTrialName,
kDurationInMsParameterName, -1);
}
int64_t GetGRCRenderProcessCPUProfilingIntervalInMs() {
return GetIntegerFieldTrialParam(kUkmPageLoadCPUUsageProfilingTrialName,
kIntervalInMsParameterName, -1);
}
bool IsPageAlmostIdleSignalEnabled() {
return base::FeatureList::IsEnabled(features::kPageAlmostIdle);
}
......
......@@ -17,8 +17,6 @@ namespace features {
// in the .cc file.
extern const SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_EXPORT base::Feature
kGlobalResourceCoordinator;
extern const SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_EXPORT base::Feature
kGRCRenderProcessCPUProfiling;
extern const SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_EXPORT base::Feature
kPageAlmostIdle;
......@@ -29,12 +27,6 @@ namespace resource_coordinator {
bool SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_EXPORT
IsResourceCoordinatorEnabled();
int64_t SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_EXPORT
GetGRCRenderProcessCPUProfilingDurationInMs();
int64_t SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_EXPORT
GetGRCRenderProcessCPUProfilingIntervalInMs();
bool SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_EXPORT
IsPageAlmostIdleSignalEnabled();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment