[PM] Add a lazy mode to V8PerFrameMemoryRequest

R=chrisha

Bug: 1080672
Change-Id: I4b24d1e77e9dcc228d606f6d484127b04ca0dadc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2339219
Commit-Queue: Joe Mason <joenotcharles@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798718}

components/performance_manager/public/v8_memory/v8_per_frame_memory_decorator.h

@@ -264,9 +264,8 @@ class V8PerFrameMemoryDecorator
   base::Value DescribeFrameNodeData(const FrameNode* node) const override;
   base::Value DescribeProcessNodeData(const ProcessNode* node) const override;
 
-  // Returns the amount of time to wait between requests for each process.
-  // Returns a zero TimeDelta if no requests should be made.
-  base::TimeDelta GetMinTimeBetweenRequestsPerProcess() const;
+  // Returns the next measurement request that should be scheduled.
+  V8PerFrameMemoryRequest* GetNextRequest() const;
 
   // Implementation details below this point.
 
@@ -288,7 +287,7 @@ class V8PerFrameMemoryDecorator
 
   Graph* graph_ = nullptr;
 
-  // List of requests sorted by sample_frequency (lowest first).
+  // List of requests sorted by min_time_between_requests (lowest first).
   std::vector<V8PerFrameMemoryRequest*> measurement_requests_;
 
   SEQUENCE_CHECKER(sequence_checker_);
@@ -296,25 +295,54 @@ class V8PerFrameMemoryDecorator
 
 class V8PerFrameMemoryRequest {
  public:
+  enum class MeasurementMode {
+    // Measurements will be taken at the next GC after a request is received.
+    // If no GC happens within a bounded time an extra GC will be scheduled.
+    kBounded,
+
+    // Measurements will only be taken at the next scheduled GC after a request
+    // is received.
+    kLazy,
+
+    kDefault = kBounded,
+  };
+
   // Creates a request but does not start the measurements. Call
   // StartMeasurement to add it to the request list.
-  explicit V8PerFrameMemoryRequest(const base::TimeDelta& sample_frequency);
+  //
+  // Measurement requests will be sent repeatedly to each process, with at
+  // least |min_time_between_requests| (which must be greater than 0) between
+  // each repetition. The next GC after each request is received will be
+  // instrumented, which adds some overhead. |mode| determines whether extra
+  // GC's can be scheduled, which would add even more overhead.
+  explicit V8PerFrameMemoryRequest(
+      const base::TimeDelta& min_time_between_requests,
+      MeasurementMode mode = MeasurementMode::kDefault);
+
+  // Creates a request and calls StartMeasurement with the given |graph| and
+  // |min_time_between_requests|, using the default measurement mode.
+  V8PerFrameMemoryRequest(const base::TimeDelta& min_time_between_requests,
+                          Graph* graph);
 
-  // Creates a request and calls StartMeasurement. This will request
-  // measurements for all ProcessNode's in |graph| with frequency
-  // |sample_frequency|.
-  V8PerFrameMemoryRequest(const base::TimeDelta& sample_frequency,
+  // Creates a request and calls StartMeasurement with the given |graph|,
+  // |min_time_between_requests|, and |mode|.
+  V8PerFrameMemoryRequest(const base::TimeDelta& min_time_between_requests,
+                          MeasurementMode mode,
                           Graph* graph);
+
   ~V8PerFrameMemoryRequest();
 
   V8PerFrameMemoryRequest(const V8PerFrameMemoryRequest&) = delete;
   V8PerFrameMemoryRequest& operator=(const V8PerFrameMemoryRequest&) = delete;
 
-  const base::TimeDelta& sample_frequency() const { return sample_frequency_; }
+  const base::TimeDelta& min_time_between_requests() const {
+    return min_time_between_requests_;
+  }
+
+  MeasurementMode mode() const { return mode_; }
 
-  // Requests measurements for all ProcessNode's in |graph| with this object's
-  // sample frequency. This must only be called once for each
-  // V8PerFrameMemoryRequest.
+  // Requests measurements for all ProcessNode's in |graph|. This must only be
+  // called once for each V8PerFrameMemoryRequest.
   void StartMeasurement(Graph* graph);
 
   // Adds/removes an observer.
@@ -326,10 +354,11 @@ class V8PerFrameMemoryRequest {
   // Private constructor for V8PerFrameMemoryRequestAnySeq. Saves
   // |off_sequence_request| as a pointer to the off-sequence object that
   // triggered the request and starts measurements with frequency
-  // |sample_frequency|.
+  // |min_time_between_requests|.
   V8PerFrameMemoryRequest(
       util::PassKey<V8PerFrameMemoryRequestAnySeq>,
-      const base::TimeDelta& sample_frequency,
+      const base::TimeDelta& min_time_between_requests,
+      MeasurementMode mode,
       base::WeakPtr<V8PerFrameMemoryRequestAnySeq> off_sequence_request);
 
   // V8PerFrameMemoryDecorator calls OnDecoratorUnregistered when it is removed
@@ -343,7 +372,8 @@ class V8PerFrameMemoryRequest {
       const ProcessNode* process_node) const;
 
  private:
-  base::TimeDelta sample_frequency_;
+  base::TimeDelta min_time_between_requests_;
+  MeasurementMode mode_;
   V8PerFrameMemoryDecorator* decorator_ = nullptr;
   base::ObserverList<V8PerFrameMemoryObserver, /*check_empty=*/true> observers_;
 
@@ -450,8 +480,11 @@ class V8PerFrameMemoryObserverAnySeq : public base::CheckedObserver {
 // Wrapper that can instantiate a V8PerFrameMemoryRequest from any sequence.
 class V8PerFrameMemoryRequestAnySeq {
  public:
+  using MeasurementMode = V8PerFrameMemoryRequest::MeasurementMode;
+
   explicit V8PerFrameMemoryRequestAnySeq(
-      const base::TimeDelta& sample_frequency);
+      const base::TimeDelta& min_time_between_requests,
+      MeasurementMode mode = MeasurementMode::kDefault);
   ~V8PerFrameMemoryRequestAnySeq();
 
   V8PerFrameMemoryRequestAnySeq(const V8PerFrameMemoryRequestAnySeq&) = delete;

components/performance_manager/v8_memory/v8_per_frame_memory_decorator.cc

@@ -48,6 +48,8 @@ class V8PerFrameMemoryDecorator::ObserverNotifier {
 
 namespace {
 
+using MeasurementMode = V8PerFrameMemoryRequest::MeasurementMode;
+
 using PerFrameUsagePtr = blink::mojom::PerFrameV8MemoryUsageDataPtr;
 
 // Comparator that generates a strict total order of PerFrameUsagePtr's when
@@ -185,7 +187,7 @@ class NodeAttachedProcessData
   void ScheduleNextMeasurement();
 
  private:
-  void StartMeasurement();
+  void StartMeasurement(MeasurementMode mode);
   void EnsureRemote();
   void OnPerFrameV8MemoryUsageData(
       blink::mojom::PerProcessV8MemoryUsageDataPtr result);
@@ -228,10 +230,14 @@ void NodeAttachedProcessData::ScheduleNextMeasurement() {
     return;
   }
 
+  V8PerFrameMemoryRequest* next_request = nullptr;
   auto* decorator =
       V8PerFrameMemoryDecorator::GetFromGraph(process_node_->GetGraph());
-  if (!decorator ||
-      decorator->GetMinTimeBetweenRequestsPerProcess().is_zero()) {
+  if (decorator) {
+    next_request = decorator->GetNextRequest();
+  }
+
+  if (!next_request) {
     // All measurements have been cancelled, or decorator was removed from
     // graph.
     state_ = State::kIdle;
@@ -243,17 +249,20 @@ void NodeAttachedProcessData::ScheduleNextMeasurement() {
   state_ = State::kWaiting;
   if (last_request_time_.is_null()) {
     // This is the first measurement. Perform it immediately.
-    StartMeasurement();
+    StartMeasurement(next_request->mode());
     return;
   }
 
+  // TODO(joenotcharles): Make sure kLazy requests can't starve kBounded
+  // requests.
   base::TimeTicks next_request_time =
-      last_request_time_ + decorator->GetMinTimeBetweenRequestsPerProcess();
-  timer_.Start(FROM_HERE, next_request_time - base::TimeTicks::Now(), this,
-               &NodeAttachedProcessData::StartMeasurement);
+      last_request_time_ + next_request->min_time_between_requests();
+  timer_.Start(FROM_HERE, next_request_time - base::TimeTicks::Now(),
+               base::BindOnce(&NodeAttachedProcessData::StartMeasurement,
+                              base::Unretained(this), next_request->mode()));
 }
 
-void NodeAttachedProcessData::StartMeasurement() {
+void NodeAttachedProcessData::StartMeasurement(MeasurementMode mode) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK_EQ(state_, State::kWaiting);
   state_ = State::kMeasuring;
@@ -268,7 +277,9 @@ void NodeAttachedProcessData::StartMeasurement() {
   // NodeAttachedProcessData when the last V8PerFrameMemoryRequest is deleted,
   // which could happen at any time.
   resource_usage_reporter_->GetPerFrameV8MemoryUsageData(
-      blink::mojom::V8PerFrameMemoryReporter::Mode::DEFAULT,
+      mode == MeasurementMode::kLazy
+          ? blink::mojom::V8PerFrameMemoryReporter::Mode::LAZY
+          : blink::mojom::V8PerFrameMemoryReporter::Mode::DEFAULT,
       base::BindOnce(&NodeAttachedProcessData::OnPerFrameV8MemoryUsageData,
                      weak_factory_.GetWeakPtr()));
 }
@@ -371,16 +382,25 @@ void SetBindV8PerFrameMemoryReporterCallbackForTesting(
 // V8PerFrameMemoryRequest
 
 V8PerFrameMemoryRequest::V8PerFrameMemoryRequest(
-    const base::TimeDelta& sample_frequency)
-    : sample_frequency_(sample_frequency) {
-  DCHECK_GT(sample_frequency_, base::TimeDelta());
+    const base::TimeDelta& min_time_between_requests,
+    MeasurementMode mode)
+    : min_time_between_requests_(min_time_between_requests), mode_(mode) {
+  DCHECK_GT(min_time_between_requests_, base::TimeDelta());
+}
+
+V8PerFrameMemoryRequest::V8PerFrameMemoryRequest(
+    const base::TimeDelta& min_time_between_requests,
+    Graph* graph)
+    : V8PerFrameMemoryRequest(min_time_between_requests,
+                              MeasurementMode::kDefault) {
+  StartMeasurement(graph);
 }
 
 V8PerFrameMemoryRequest::V8PerFrameMemoryRequest(
-    const base::TimeDelta& sample_frequency,
+    const base::TimeDelta& min_time_between_requests,
+    MeasurementMode mode,
     Graph* graph)
-    : sample_frequency_(sample_frequency) {
-  DCHECK_GT(sample_frequency_, base::TimeDelta());
+    : V8PerFrameMemoryRequest(min_time_between_requests, mode) {
   StartMeasurement(graph);
 }
 
@@ -388,12 +408,13 @@ V8PerFrameMemoryRequest::V8PerFrameMemoryRequest(
 // sequence.
 V8PerFrameMemoryRequest::V8PerFrameMemoryRequest(
     util::PassKey<V8PerFrameMemoryRequestAnySeq>,
-    const base::TimeDelta& sample_frequency,
+    const base::TimeDelta& min_time_between_requests,
+    MeasurementMode mode,
     base::WeakPtr<V8PerFrameMemoryRequestAnySeq> off_sequence_request)
-    : sample_frequency_(sample_frequency),
-      off_sequence_request_(std::move(off_sequence_request)),
-      off_sequence_request_sequence_(base::SequencedTaskRunnerHandle::Get()) {
+    : V8PerFrameMemoryRequest(min_time_between_requests, mode) {
   DETACH_FROM_SEQUENCE(sequence_checker_);
+  off_sequence_request_ = std::move(off_sequence_request);
+  off_sequence_request_sequence_ = base::SequencedTaskRunnerHandle::Get();
   // Unretained is safe since |this| will be destroyed on the graph sequence.
   PerformanceManager::CallOnGraph(
       FROM_HERE, base::BindOnce(&V8PerFrameMemoryRequest::StartMeasurement,
@@ -586,12 +607,10 @@ base::Value V8PerFrameMemoryDecorator::DescribeProcessNodeData(
   return dict;
 }
 
-base::TimeDelta V8PerFrameMemoryDecorator::GetMinTimeBetweenRequestsPerProcess()
-    const {
+V8PerFrameMemoryRequest* V8PerFrameMemoryDecorator::GetNextRequest() const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  return measurement_requests_.empty()
-             ? base::TimeDelta()
-             : measurement_requests_.front()->sample_frequency();
+  return measurement_requests_.empty() ? nullptr
+                                       : measurement_requests_.front();
 }
 
 void V8PerFrameMemoryDecorator::AddMeasurementRequest(
@@ -607,7 +626,14 @@ void V8PerFrameMemoryDecorator::AddMeasurementRequest(
   for (std::vector<V8PerFrameMemoryRequest*>::const_iterator it =
            measurement_requests_.begin();
        it != measurement_requests_.end(); ++it) {
-    if (request->sample_frequency() < (*it)->sample_frequency()) {
+    if (request->min_time_between_requests() <
+            (*it)->min_time_between_requests() ||
+        // Make sure bounded request sort before lazy requests so that they
+        // aren't starved.
+        (request->min_time_between_requests() ==
+             (*it)->min_time_between_requests() &&
+         request->mode() ==
+             V8PerFrameMemoryRequest::MeasurementMode::kBounded)) {
       measurement_requests_.insert(it, request);
       UpdateProcessMeasurementSchedules();
       return;
@@ -636,8 +662,8 @@ void V8PerFrameMemoryDecorator::UpdateProcessMeasurementSchedules() const {
   for (size_t i = 1; i < measurement_requests_.size(); ++i) {
     DCHECK(measurement_requests_[i - 1]);
     DCHECK(measurement_requests_[i]);
-    DCHECK_LE(measurement_requests_[i - 1]->sample_frequency(),
-              measurement_requests_[i]->sample_frequency());
+    DCHECK_LE(measurement_requests_[i - 1]->min_time_between_requests(),
+              measurement_requests_[i]->min_time_between_requests());
   }
 #endif
   for (const ProcessNode* node : graph_->GetAllProcessNodes()) {
@@ -666,7 +692,8 @@ void V8PerFrameMemoryDecorator::NotifyObserversOnMeasurementAvailable(
 // V8PerFrameMemoryRequestAnySeq
 
 V8PerFrameMemoryRequestAnySeq::V8PerFrameMemoryRequestAnySeq(
-    const base::TimeDelta& sample_frequency) {
+    const base::TimeDelta& min_time_between_requests,
+    MeasurementMode mode) {
   // |request_| must be initialized in the constructor body so that
   // |weak_factory_| is completely constructed.
   //
@@ -674,8 +701,8 @@ V8PerFrameMemoryRequestAnySeq::V8PerFrameMemoryRequestAnySeq(
   // constructor. After construction the V8PerFrameMemoryRequest must only be
   // accessed on the graph sequence.
   request_ = base::WrapUnique(new V8PerFrameMemoryRequest(
-      util::PassKey<V8PerFrameMemoryRequestAnySeq>(), sample_frequency,
-      weak_factory_.GetWeakPtr()));
+      util::PassKey<V8PerFrameMemoryRequestAnySeq>(), min_time_between_requests,
+      mode, weak_factory_.GetWeakPtr()));
 }
 
 V8PerFrameMemoryRequestAnySeq::~V8PerFrameMemoryRequestAnySeq() {