[PM] Expose EAGER measurement mode for testing.

This adds MeasurementMode::kEagerForTesting, which can only be used if
SetEagerMemoryMeasurementEnabledForTesting is called. This mode causes
an instrumented GC as soon as the measurement request is sent, so the
measurement times are more predictable in tests.

R=fdoray

Bug: 1122131
Change-Id: I7901f3021cea1e133a9c20c94ad7082bad114fd5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2386775
Commit-Queue: Joe Mason <joenotcharles@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Auto-Submit: Joe Mason <joenotcharles@chromium.org>
Cr-Commit-Position: refs/heads/master@{#804011}

components/performance_manager/public/v8_memory/v8_per_frame_memory_decorator.h

@@ -269,8 +269,8 @@ class V8PerFrameMemoryDecorator
   // Returns the next measurement request that should be scheduled.
   V8PerFrameMemoryRequest* GetNextRequest() const;
 
-  // Returns the next measurement request with mode kBounded that should be
-  // scheduled.
+  // Returns the next measurement request with mode kBounded or
+  // kEagerForTesting that should be scheduled.
   V8PerFrameMemoryRequest* GetNextBoundedRequest() const;
 
   // Implementation details below this point.
@@ -311,6 +311,12 @@ class V8PerFrameMemoryRequest {
     // is received.
     kLazy,
 
+    // Measurements will be taken immediately when a request is received. This
+    // causes an extra GC so should only be done in tests. Attempts to use this
+    // mode will DCHECK if SetEagerMemoryMeasurementEnabledForTesting was not
+    // called.
+    kEagerForTesting,
+
     kDefault = kBounded,
   };
 
@@ -545,6 +551,11 @@ using BindV8DetailedMemoryReporterCallback = base::RepeatingCallback<void(
 void SetBindV8DetailedMemoryReporterCallbackForTesting(
     BindV8DetailedMemoryReporterCallback* callback);
 
+// Enables or disables MeasurementMode::kEagerModeForTesting. Creating eager
+// measurement requests can have a high performance penalty so this should only
+// be enabled in tests.
+void SetEagerMemoryMeasurementEnabledForTesting(bool enable);
+
 }  // namespace internal
 
 }  // namespace v8_memory

components/performance_manager/v8_memory/v8_per_frame_memory_decorator.cc

@@ -60,8 +60,23 @@ void BindReceiverOnUIThread(
   }
 }
 
+bool IsMeasurementBounded(MeasurementMode mode) {
+  switch (mode) {
+    case MeasurementMode::kLazy:
+      return false;
+    case MeasurementMode::kBounded:
+      return true;
+    case MeasurementMode::kEagerForTesting:
+      return true;
+  }
+}
+
 internal::BindV8DetailedMemoryReporterCallback* g_test_bind_callback = nullptr;
 
+#if DCHECK_IS_ON()
+bool g_test_eager_measurement_requests_enabled = false;
+#endif
+
 // Per-frame memory measurement involves the following classes that live on the
 // PM sequence:
 //
@@ -158,7 +173,7 @@ class NodeAttachedProcessData
  private:
   void StartMeasurement(MeasurementMode mode);
   void ScheduleUpgradeToBoundedMeasurement();
-  void UpgradeToBoundedMeasurementIfNeeded();
+  void UpgradeToBoundedMeasurementIfNeeded(MeasurementMode bounded_mode);
   void EnsureRemote();
   void OnV8MemoryUsage(blink::mojom::PerProcessV8MemoryUsagePtr result);
 
@@ -255,15 +270,15 @@ void NodeAttachedProcessData::ScheduleNextMeasurement() {
 
 void NodeAttachedProcessData::StartMeasurement(MeasurementMode mode) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  if (mode == MeasurementMode::kLazy) {
+  if (IsMeasurementBounded(mode)) {
+    DCHECK(state_ == State::kWaiting || state_ == State::kMeasuringLazy);
+    state_ = State::kMeasuringBounded;
+  } else {
     DCHECK_EQ(state_, State::kWaiting);
     state_ = State::kMeasuringLazy;
     // Ensure this lazy measurement doesn't starve any bounded measurements in
     // the queue.
     ScheduleUpgradeToBoundedMeasurement();
-  } else {
-    DCHECK(state_ == State::kWaiting || state_ == State::kMeasuringLazy);
-    state_ = State::kMeasuringBounded;
   }
 
   last_request_time_ = base::TimeTicks::Now();
@@ -276,11 +291,24 @@ void NodeAttachedProcessData::StartMeasurement(MeasurementMode mode) {
   // which should happen after renderers are destroyed). Should clean up
   // NodeAttachedProcessData when the last V8PerFrameMemoryRequest is deleted,
   // which could happen at any time.
+  blink::mojom::V8DetailedMemoryReporter::Mode mojo_mode;
+  switch (mode) {
+    case MeasurementMode::kLazy:
+      mojo_mode = blink::mojom::V8DetailedMemoryReporter::Mode::LAZY;
+      break;
+    case MeasurementMode::kBounded:
+      mojo_mode = blink::mojom::V8DetailedMemoryReporter::Mode::DEFAULT;
+      break;
+    case MeasurementMode::kEagerForTesting:
+#if DCHECK_IS_ON()
+      DCHECK(g_test_eager_measurement_requests_enabled);
+#endif
+      mojo_mode = blink::mojom::V8DetailedMemoryReporter::Mode::EAGER;
+      break;
+  }
+
   resource_usage_reporter_->GetV8MemoryUsage(
-      mode == MeasurementMode::kLazy
-          ? blink::mojom::V8DetailedMemoryReporter::Mode::LAZY
-          : blink::mojom::V8DetailedMemoryReporter::Mode::DEFAULT,
-      base::BindOnce(&NodeAttachedProcessData::OnV8MemoryUsage,
+      mojo_mode, base::BindOnce(&NodeAttachedProcessData::OnV8MemoryUsage,
                                 weak_factory_.GetWeakPtr()));
 }
 
@@ -306,16 +334,18 @@ void NodeAttachedProcessData::ScheduleUpgradeToBoundedMeasurement() {
       FROM_HERE, bounded_request_time - base::TimeTicks::Now(),
       base::BindOnce(
           &NodeAttachedProcessData::UpgradeToBoundedMeasurementIfNeeded,
-          base::Unretained(this)));
+          base::Unretained(this), bounded_request->mode()));
 }
 
-void NodeAttachedProcessData::UpgradeToBoundedMeasurementIfNeeded() {
+void NodeAttachedProcessData::UpgradeToBoundedMeasurementIfNeeded(
+    MeasurementMode bounded_mode) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (state_ != State::kMeasuringLazy) {
     // State changed before timer expired.
     return;
   }
-  StartMeasurement(MeasurementMode::kBounded);
+  DCHECK(IsMeasurementBounded(bounded_mode));
+  StartMeasurement(bounded_mode);
 }
 
 void NodeAttachedProcessData::OnV8MemoryUsage(
@@ -426,6 +456,12 @@ void SetBindV8DetailedMemoryReporterCallbackForTesting(
   g_test_bind_callback = callback;
 }
 
+void SetEagerMemoryMeasurementEnabledForTesting(bool enabled) {
+#if DCHECK_IS_ON()
+  g_test_eager_measurement_requests_enabled = enabled;
+#endif
+}
+
 }  // namespace internal
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -435,7 +471,11 @@ V8PerFrameMemoryRequest::V8PerFrameMemoryRequest(
     const base::TimeDelta& min_time_between_requests,
     MeasurementMode mode)
     : min_time_between_requests_(min_time_between_requests), mode_(mode) {
+#if DCHECK_IS_ON()
   DCHECK_GT(min_time_between_requests_, base::TimeDelta());
+  DCHECK(mode != MeasurementMode::kEagerForTesting ||
+         g_test_eager_measurement_requests_enabled);
+#endif
 }
 
 V8PerFrameMemoryRequest::V8PerFrameMemoryRequest(
@@ -694,8 +734,8 @@ void V8PerFrameMemoryDecorator::AddMeasurementRequest(
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(request);
   std::vector<V8PerFrameMemoryRequest*>& measurement_requests =
-      request->mode() == MeasurementMode::kLazy ? lazy_measurement_requests_
-                                                : bounded_measurement_requests_;
+      IsMeasurementBounded(request->mode()) ? bounded_measurement_requests_
+                                            : lazy_measurement_requests_;
   DCHECK(!base::Contains(measurement_requests, request))
       << "V8PerFrameMemoryRequest object added twice";
   // Each user of this decorator is expected to issue a single
@@ -720,9 +760,9 @@ void V8PerFrameMemoryDecorator::RemoveMeasurementRequest(
     V8PerFrameMemoryRequest* request) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(request);
-  size_t num_erased = base::Erase(request->mode() == MeasurementMode::kLazy
-                                      ? lazy_measurement_requests_
-                                      : bounded_measurement_requests_,
+  size_t num_erased = base::Erase(IsMeasurementBounded(request->mode())
+                                      ? bounded_measurement_requests_
+                                      : lazy_measurement_requests_,
                                   request);
   DCHECK_EQ(num_erased, 1ULL);
   UpdateProcessMeasurementSchedules();
@@ -736,18 +776,20 @@ void V8PerFrameMemoryDecorator::UpdateProcessMeasurementSchedules() const {
   // ScheduleNextMeasurement.
   auto check_invariants =
       [](const std::vector<V8PerFrameMemoryRequest*>& measurement_requests,
-         MeasurementMode mode) {
+         bool is_bounded) {
         for (size_t i = 1; i < measurement_requests.size(); ++i) {
           DCHECK(measurement_requests[i - 1]);
           DCHECK(measurement_requests[i]);
-          DCHECK_EQ(measurement_requests[i - 1]->mode(), mode);
-          DCHECK_EQ(measurement_requests[i]->mode(), mode);
+          DCHECK_EQ(IsMeasurementBounded(measurement_requests[i - 1]->mode()),
+                    is_bounded);
+          DCHECK_EQ(IsMeasurementBounded(measurement_requests[i]->mode()),
+                    is_bounded);
           DCHECK_LE(measurement_requests[i - 1]->min_time_between_requests(),
                     measurement_requests[i]->min_time_between_requests());
         }
       };
-  check_invariants(bounded_measurement_requests_, MeasurementMode::kBounded);
-  check_invariants(lazy_measurement_requests_, MeasurementMode::kLazy);
+  check_invariants(bounded_measurement_requests_, true);
+  check_invariants(lazy_measurement_requests_, false);
 #endif
   for (const ProcessNode* node : graph_->GetAllProcessNodes()) {
     NodeAttachedProcessData* process_data = NodeAttachedProcessData::Get(node);

components/performance_manager/v8_memory/v8_per_frame_memory_decorator_unittest.cc

@@ -5,6 +5,7 @@
 #include "components/performance_manager/public/v8_memory/v8_per_frame_memory_decorator.h"
 
 #include <memory>
+#include <tuple>
 #include <utility>
 
 #include "base/bind.h"
@@ -100,9 +101,16 @@ class LenientMockV8PerFrameMemoryObserverAnySeq
                const V8PerFrameMemoryObserverAnySeq::FrameDataMap& frame_data),
               (override));
 };
+
 using MockV8PerFrameMemoryObserverAnySeq =
     testing::StrictMock<LenientMockV8PerFrameMemoryObserverAnySeq>;
 
+// The mode enum used in the API.
+using MeasurementMode = V8PerFrameMemoryRequest::MeasurementMode;
+
+// The mode enum used in test expectations.
+using ExpectedMode = MockV8DetailedMemoryReporter::Mode;
+
 class V8PerFrameMemoryDecoratorTestBase {
  public:
   static constexpr base::TimeDelta kMinTimeBetweenRequests =
@@ -141,11 +149,10 @@ class V8PerFrameMemoryDecoratorTestBase {
       base::RepeatingCallback<
           void(MockV8DetailedMemoryReporter::GetV8MemoryUsageCallback callback)>
           responder,
-      MockV8DetailedMemoryReporter::Mode expected_mode =
-          MockV8DetailedMemoryReporter::Mode::DEFAULT) {
+      ExpectedMode expected_mode = ExpectedMode::DEFAULT) {
     EXPECT_CALL(*mock_reporter, GetV8MemoryUsage(expected_mode, _))
         .WillOnce([this, responder](
-                      MockV8DetailedMemoryReporter::Mode mode,
+                      ExpectedMode mode,
                       MockV8DetailedMemoryReporter::GetV8MemoryUsageCallback
                           callback) {
           this->last_query_time_ = base::TimeTicks::Now();
@@ -155,8 +162,7 @@ class V8PerFrameMemoryDecoratorTestBase {
 
   void ExpectQueryAndReply(MockV8DetailedMemoryReporter* mock_reporter,
                            blink::mojom::PerProcessV8MemoryUsagePtr data,
-                           MockV8DetailedMemoryReporter::Mode expected_mode =
-                               MockV8DetailedMemoryReporter::Mode::DEFAULT) {
+                           ExpectedMode expected_mode = ExpectedMode::DEFAULT) {
     ExpectQuery(
         mock_reporter,
         base::BindRepeating(&V8PerFrameMemoryDecoratorTestBase::ReplyWithData,
@@ -168,8 +174,7 @@ class V8PerFrameMemoryDecoratorTestBase {
       MockV8DetailedMemoryReporter* mock_reporter,
       const base::TimeDelta& delay,
       blink::mojom::PerProcessV8MemoryUsagePtr data,
-      MockV8DetailedMemoryReporter::Mode expected_mode =
-          MockV8DetailedMemoryReporter::Mode::DEFAULT) {
+      ExpectedMode expected_mode = ExpectedMode::DEFAULT) {
     ExpectQuery(mock_reporter,
                 base::BindRepeating(
                     &V8PerFrameMemoryDecoratorTestBase::DelayedReplyWithData,
@@ -181,8 +186,7 @@ class V8PerFrameMemoryDecoratorTestBase {
       MockV8DetailedMemoryReporter* mock_reporter,
       blink::mojom::PerProcessV8MemoryUsagePtr data,
       RenderProcessHostId expected_process_id = kTestProcessID,
-      MockV8DetailedMemoryReporter::Mode expected_mode =
-          MockV8DetailedMemoryReporter::Mode::DEFAULT) {
+      ExpectedMode expected_mode = ExpectedMode::DEFAULT) {
     // Wrap the move-only |data| in a callback for the expectation below.
     ExpectQueryAndReply(mock_reporter, std::move(data), expected_mode);
 
@@ -257,6 +261,32 @@ class V8PerFrameMemoryDecoratorTest : public GraphTestHarness,
   }
 };
 
+// kBounded mode and kEagerForTesting mode behave identically as far as
+// V8PerFrameMemoryDecorator is concerned. (The differences are all on the
+// renderer side.) So mode tests hardcode kLazy mode and use a parameter to
+// choose which of the two to use for bounded mode.
+class V8PerFrameMemoryDecoratorModeTest
+    : public V8PerFrameMemoryDecoratorTest,
+      public ::testing::WithParamInterface<
+          std::pair<MeasurementMode, ExpectedMode>> {
+ public:
+  V8PerFrameMemoryDecoratorModeTest() {
+    internal::SetEagerMemoryMeasurementEnabledForTesting(true);
+    std::tie(bounded_mode_, expected_bounded_mode_) = GetParam();
+  }
+
+  ~V8PerFrameMemoryDecoratorModeTest() override {
+    internal::SetEagerMemoryMeasurementEnabledForTesting(false);
+  }
+
+ protected:
+  // The mode that will be used for bounded requests.
+  MeasurementMode bounded_mode_;
+
+  // The expected mojo mode parameter for bounded requests.
+  ExpectedMode expected_bounded_mode_;
+};
+
 using V8PerFrameMemoryDecoratorDeathTest = V8PerFrameMemoryDecoratorTest;
 
 class V8PerFrameMemoryRequestAnySeqTest
@@ -602,18 +632,17 @@ TEST_F(V8PerFrameMemoryDecoratorTest, PerFrameDataIsDistributed) {
                        ->unassociated_v8_bytes_used());
 }
 
-TEST_F(V8PerFrameMemoryDecoratorTest, LazyRequests) {
+TEST_P(V8PerFrameMemoryDecoratorModeTest, LazyRequests) {
   constexpr base::TimeDelta kLazyRequestLength =
       base::TimeDelta::FromSeconds(30);
-  V8PerFrameMemoryRequest lazy_request(
-      kLazyRequestLength, V8PerFrameMemoryRequest::MeasurementMode::kLazy,
-      graph());
+  V8PerFrameMemoryRequest lazy_request(kLazyRequestLength,
+                                       MeasurementMode::kLazy, graph());
 
   MockV8DetailedMemoryReporter reporter;
   {
     auto data = NewPerProcessV8MemoryUsage(1);
     ExpectBindAndRespondToQuery(&reporter, std::move(data), kTestProcessID,
-                                MockV8DetailedMemoryReporter::Mode::LAZY);
+                                ExpectedMode::LAZY);
   }
 
   auto process = CreateNode<ProcessNodeImpl>(
@@ -628,14 +657,13 @@ TEST_F(V8PerFrameMemoryDecoratorTest, LazyRequests) {
   constexpr base::TimeDelta kLongBoundedRequestLength =
       base::TimeDelta::FromSeconds(45);
   V8PerFrameMemoryRequest long_bounded_request(kLongBoundedRequestLength,
-                                               graph());
+                                               bounded_mode_, graph());
   auto* decorator = V8PerFrameMemoryDecorator::GetFromGraph(graph());
   ASSERT_TRUE(decorator);
   ASSERT_TRUE(decorator->GetNextRequest());
   EXPECT_EQ(decorator->GetNextRequest()->min_time_between_requests(),
             kLazyRequestLength);
-  EXPECT_EQ(decorator->GetNextRequest()->mode(),
-            V8PerFrameMemoryRequest::MeasurementMode::kLazy);
+  EXPECT_EQ(decorator->GetNextRequest()->mode(), MeasurementMode::kLazy);
   {
     // Next lazy request sent after 30 sec + 10 sec delay until reply = 40 sec
     // until reply arrives. kLongBoundedRequestLength > 40 sec so the reply
@@ -643,8 +671,7 @@ TEST_F(V8PerFrameMemoryDecoratorTest, LazyRequests) {
     auto data = NewPerProcessV8MemoryUsage(1);
     data->isolates[0]->unassociated_bytes_used = 1U;
     ExpectQueryAndDelayReply(&reporter, base::TimeDelta::FromSeconds(10),
-                             std::move(data),
-                             MockV8DetailedMemoryReporter::Mode::LAZY);
+                             std::move(data), ExpectedMode::LAZY);
   }
 
   // Wait long enough for the upgraded request to be sent, to verify that it
@@ -655,12 +682,11 @@ TEST_F(V8PerFrameMemoryDecoratorTest, LazyRequests) {
   constexpr base::TimeDelta kUpgradeRequestLength =
       base::TimeDelta::FromSeconds(40);
   V8PerFrameMemoryRequest bounded_request_upgrade(kUpgradeRequestLength,
-                                                  graph());
+                                                  bounded_mode_, graph());
   ASSERT_TRUE(decorator->GetNextRequest());
   EXPECT_EQ(decorator->GetNextRequest()->min_time_between_requests(),
             kLazyRequestLength);
-  EXPECT_EQ(decorator->GetNextRequest()->mode(),
-            V8PerFrameMemoryRequest::MeasurementMode::kLazy);
+  EXPECT_EQ(decorator->GetNextRequest()->mode(), MeasurementMode::kLazy);
 
   {
     ::testing::InSequence seq;
@@ -670,13 +696,11 @@ TEST_F(V8PerFrameMemoryDecoratorTest, LazyRequests) {
     auto data = NewPerProcessV8MemoryUsage(1);
     data->isolates[0]->unassociated_bytes_used = 2U;
     ExpectQueryAndDelayReply(&reporter, base::TimeDelta::FromSeconds(10),
-                             std::move(data),
-                             MockV8DetailedMemoryReporter::Mode::LAZY);
+                             std::move(data), ExpectedMode::LAZY);
 
     auto data2 = NewPerProcessV8MemoryUsage(1);
     data2->isolates[0]->unassociated_bytes_used = 3U;
-    ExpectQueryAndReply(&reporter, std::move(data2),
-                        MockV8DetailedMemoryReporter::Mode::DEFAULT);
+    ExpectQueryAndReply(&reporter, std::move(data2), expected_bounded_mode_);
   }
 
   // Wait long enough for the upgraded request to be sent.
@@ -689,14 +713,22 @@ TEST_F(V8PerFrameMemoryDecoratorTest, LazyRequests) {
 
   // Bounded requests should be preferred over lazy requests with the same
   // min_time_between_requests.
-  V8PerFrameMemoryRequest short_bounded_request(kLazyRequestLength, graph());
+  V8PerFrameMemoryRequest short_bounded_request(kLazyRequestLength,
+                                                bounded_mode_, graph());
   ASSERT_TRUE(decorator->GetNextRequest());
   EXPECT_EQ(decorator->GetNextRequest()->min_time_between_requests(),
             kLazyRequestLength);
-  EXPECT_EQ(decorator->GetNextRequest()->mode(),
-            V8PerFrameMemoryRequest::MeasurementMode::kBounded);
+  EXPECT_EQ(decorator->GetNextRequest()->mode(), bounded_mode_);
 }
 
+INSTANTIATE_TEST_SUITE_P(
+    AllBoundedModes,
+    V8PerFrameMemoryDecoratorModeTest,
+    testing::Values(std::make_pair(MeasurementMode::kBounded,
+                                   ExpectedMode::DEFAULT),
+                    std::make_pair(MeasurementMode::kEagerForTesting,
+                                   ExpectedMode::EAGER)));
+
 TEST_F(V8PerFrameMemoryDecoratorTest, MeasurementRequestsSorted) {
   // Create some queries with different sample frequencies.
   constexpr base::TimeDelta kShortInterval(kMinTimeBetweenRequests);
@@ -1283,6 +1315,19 @@ TEST_F(V8PerFrameMemoryDecoratorDeathTest, EnforceObserversRemoved) {
   });
 }
 
+TEST_F(V8PerFrameMemoryDecoratorDeathTest, EagerModeDisallowed) {
+  // Not allowed to use kEagerForTesting mode without calling
+  // SetEagerMemoryMeasurementEnabledForTesting.
+  EXPECT_DCHECK_DEATH({
+    V8PerFrameMemoryRequest memory_request(kMinTimeBetweenRequests,
+                                           MeasurementMode::kEagerForTesting);
+  });
+  EXPECT_DCHECK_DEATH({
+    V8PerFrameMemoryRequestAnySeq memory_request(
+        kMinTimeBetweenRequests, MeasurementMode::kEagerForTesting);
+  });
+}
+
 TEST_F(V8PerFrameMemoryRequestAnySeqTest, RequestIsSequenceSafe) {
   // Precondition: CallOnGraph must run on a different sequence.  Note that all
   // tasks passed to CallOnGraph will only run when run_loop.Run() is called