[compile hints] Use InteractiveDetector for triggering compile hint g…

…eneration

It's more robust than the previous timer-based approach, e.g., it
scales better to different type of web sites, generating load-specific
compile hints for both sites which load slowly and which load fast.

Bug: chromium:1406506
Change-Id: I7b4c21345210d493bbff2d0795da2d5474183a48
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4353132
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1120486}

third_party/blink/renderer/bindings/core/v8/v8_compile_hints.cc

@@ -13,6 +13,7 @@
 #include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_core.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"
 #include "third_party/blink/renderer/core/frame/frame.h"
+#include "third_party/blink/renderer/core/frame/local_frame.h"
 #include "third_party/blink/renderer/core/page/page.h"
 #include "third_party/blink/renderer/platform/bindings/script_state.h"
 #include "third_party/blink/renderer/platform/wtf/bloom_filter.h"
@@ -27,6 +28,8 @@ constexpr int kBloomFilterInt32Count = 512;
 
 std::atomic<bool> V8CompileHints::data_generated_for_this_process_ = false;
 
+V8CompileHints::V8CompileHints(Page* page) : page_(page) {}
+
 void V8CompileHints::RecordScript(Frame* frame,
                                   ExecutionContext* execution_context,
                                   const v8::Local<v8::Script> script,
@@ -72,77 +75,50 @@ void V8CompileHints::RecordScript(Frame* frame,
 
   scripts_.emplace_back(v8::Global<v8::Script>(isolate, script));
   script_name_hashes_.emplace_back(script_name_hash);
-
-  ScheduleDataGenerationIfNeeded(frame, execution_context);
-}
-
-namespace {
-void DelayedDataGenerationTask(Page* page,
-                               ExecutionContext* execution_context) {
-  page->GetV8CompileHints().GenerateData(execution_context);
 }
-}  // namespace
-
-void V8CompileHints::ScheduleDataGenerationIfNeeded(
-    Frame* frame,
-    ExecutionContext* execution_context) {
-  // We need to use the outermost main frame's ExecutionContext for retrieving
-  // the UkmRecorder for sending the data. This means that if the main frame
-  // doesn't run scripts, compile hints from the non-main frames won't be sent.
-  // TODO(chromium:1406506): Relax that restriction.
-  if (!frame->IsOutermostMainFrame()) {
-    return;
-  }
 
-  DCHECK(state_ == State::kInitial ||
-         state_ == State::kDataGenerationScheduled);
-  if (state_ == State::kDataGenerationScheduled) {
+void V8CompileHints::GenerateData() {
+  // Call FeatureList::IsEnabled only once.
+  static bool compile_hints_enabled =
+      base::FeatureList::IsEnabled(features::kProduceCompileHints);
+  if (!compile_hints_enabled) {
     return;
   }
 
-  state_ = State::kDataGenerationScheduled;
-
-  // Schedule a task for moving the data to UKM. For now, we use a simple timer
-  // instead of a more complicated "page loaded" event, but this should be good
-  // enough for our purpose.
-
-  auto delay =
-      base::Milliseconds(features::kProduceCompileHintsOnIdleDelayParam.Get());
-
-  execution_context->GetTaskRunner(TaskType::kIdleTask)
-      ->PostDelayedTask(FROM_HERE,
-                        WTF::BindOnce(&DelayedDataGenerationTask,
-                                      WrapPersistent(frame->GetPage()),
-                                      WrapPersistent(execution_context)),
-                        delay);
-}
-
-void V8CompileHints::GenerateData(ExecutionContext* execution_context) {
+  // Guard against this function getting called repeatedly.
   if (state_ == State::kDataGenerationFinished) {
-    // This only happens when: 1) some other Page generated data 2)
-    // this V8CompileHints object got notified of a script 3) it realized that
-    // some other Page has already generated data 4) the data generation task
-    // which was already scheduled ran.
-    DCHECK(data_generated_for_this_process_);
     return;
   }
 
   // Stop logging script executions for this page.
   state_ = State::kDataGenerationFinished;
 
   if (!data_generated_for_this_process_) {
-    data_generated_for_this_process_ = SendDataToUkm(execution_context);
+    data_generated_for_this_process_ = SendDataToUkm();
   }
 
   ClearData();
 }
 
+void V8CompileHints::Trace(Visitor* visitor) const {
+  visitor->Trace(page_);
+}
+
 void V8CompileHints::ClearData() {
   scripts_.clear();
   script_name_hashes_.clear();
 }
 
-bool V8CompileHints::SendDataToUkm(ExecutionContext* execution_context) {
+bool V8CompileHints::SendDataToUkm() {
+  Frame* main_frame = page_->MainFrame();
+  // Because of OOPIF, the main frame is not necessarily a LocalFrame. We cannot
+  // generate good compile hints for those pages, so skip sending them.
+  if (!main_frame->IsLocalFrame()) {
+    return false;
+  }
+  ScriptState* script_state =
+      ToScriptStateForMainWorld(DynamicTo<LocalFrame>(main_frame));
+  ExecutionContext* execution_context = ExecutionContext::From(script_state);
   v8::Isolate* isolate = execution_context->GetIsolate();
   v8::HandleScope handle_scope(isolate);
   int total_funcs = 0;

third_party/blink/renderer/bindings/core/v8/v8_compile_hints.h

@@ -6,34 +6,37 @@
 #define THIRD_PARTY_BLINK_RENDERER_BINDINGS_CORE_V8_V8_COMPILE_HINTS_H_
 
 #include "third_party/blink/renderer/bindings/buildflags.h"
+#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
 
 #if BUILDFLAG(ENABLE_V8_COMPILE_HINTS)
 
+#include "third_party/blink/renderer/platform/heap/member.h"
 #include "third_party/blink/renderer/platform/wtf/vector.h"
 #include "v8/include/v8.h"
 
 namespace blink {
 
 class ExecutionContext;
 class Frame;
+class Page;
 class ScriptState;
 
-class V8CompileHints {
+class V8CompileHints : public GarbageCollected<V8CompileHints> {
  public:
-  // Notifies V8CompileHints of the existence of `script`. Also schedules data
-  // generation to happen later.
+  explicit V8CompileHints(Page* page);
+  // Notifies V8CompileHints of the existence of `script`
   void RecordScript(Frame* frame,
                     ExecutionContext* execution_context,
                     const v8::Local<v8::Script> script,
                     ScriptState* script_state);
 
-  void GenerateData(ExecutionContext* execution_context);
+  void GenerateData();
+
+  void Trace(Visitor* visitor) const;
 
  private:
   void ClearData();
-  void ScheduleDataGenerationIfNeeded(Frame* frame,
-                                      ExecutionContext* execution_context);
-  bool SendDataToUkm(ExecutionContext* execution_context);
+  bool SendDataToUkm();
   static void AddNoise(unsigned* data);
 
   WTF::Vector<v8::Global<v8::Script>> scripts_;
@@ -42,10 +45,9 @@ class V8CompileHints {
   enum class State {
     kInitial,
 
-    // Task fro data generation has been scheduled.
-    kDataGenerationScheduled,
-
-    // Task for data generation has ran.
+    // We've tried once to send the data to UKM (but we didn't necessarily send
+    // it successfully; e.g., because of throttling or because we didn't have
+    // enough data).
     kDataGenerationFinished
   };
   State state_ = State::kInitial;
@@ -56,6 +58,8 @@ class V8CompileHints {
   // we get enough data from another page. Use std::atomic to be future proof
   // in case we start generating compile hints from Workers.
   static std::atomic<bool> data_generated_for_this_process_;
+
+  Member<Page> page_;
 };
 
 }  // namespace blink
@@ -64,13 +68,19 @@ class V8CompileHints {
 
 namespace blink {
 
+class Page;
+
 // A minimal implementation for platforms which don't enable compile hints.
-class V8CompileHints {
+class V8CompileHints : public GarbageCollected<V8CompileHints> {
  public:
-  V8CompileHints() = default;
+  explicit V8CompileHints(Page* page) {}
 
   V8CompileHints(const V8CompileHints&) = delete;
   V8CompileHints& operator=(const V8CompileHints&) = delete;
+
+  void GenerateData() {}
+
+  void Trace(Visitor* visitor) const {}
 };
 
 }  // namespace blink

third_party/blink/renderer/core/dom/scripted_idle_task_controller_test.cc

@@ -124,6 +124,7 @@ class IdleTaskControllerFrameScheduler : public FrameScheduler {
   void DidCommitProvisionalLoad(bool, FrameScheduler::NavigationType) override {
   }
   void OnFirstContentfulPaintInMainFrame() override {}
+  void OnMainFrameInteractive() override {}
   void OnFirstMeaningfulPaint() override {}
   void OnLoad() override {}
   bool IsExemptFromBudgetBasedThrottling() const override { return false; }

third_party/blink/renderer/core/frame/local_frame.cc

@@ -2428,6 +2428,13 @@ void LocalFrame::OnTaskCompleted(base::TimeTicks start_time,
                                     desired_execution_time, this);
   }
 }
+
+void LocalFrame::MainFrameInteractive() {
+  if (Page* page = GetPage()) {
+    page->GetV8CompileHints().GenerateData();
+  }
+}
+
 mojom::blink::ReportingServiceProxy* LocalFrame::GetReportingService() {
   return mojo_handler_->ReportingService();
 }

third_party/blink/renderer/core/frame/local_frame.h

@@ -922,6 +922,7 @@ class CORE_EXPORT LocalFrame final
   void OnTaskCompleted(base::TimeTicks start_time,
                        base::TimeTicks end_time,
                        base::TimeTicks desired_execution_time) override;
+  void MainFrameInteractive() override;
 
   // Activates the user activation states of this frame and all its ancestors.
   //

third_party/blink/renderer/core/loader/interactive_detector.cc

@@ -610,6 +610,10 @@ void InteractiveDetector::OnTimeToInteractiveDetected() {
       });
 
   long_tasks_.clear();
+
+  if (frame != nullptr && frame->IsMainFrame() && frame->GetFrameScheduler()) {
+    frame->GetFrameScheduler()->OnMainFrameInteractive();
+  }
 }
 
 base::TimeDelta InteractiveDetector::ComputeTotalBlockingTime() {

third_party/blink/renderer/core/page/page.cc

@@ -217,7 +217,8 @@ Page::Page(base::PassKey<Page>,
       next_related_page_(this),
       prev_related_page_(this),
       autoplay_flags_(0),
-      web_text_autosizer_page_info_({0, 0, 1.f}) {
+      web_text_autosizer_page_info_({0, 0, 1.f}),
+      v8_compile_hints_(MakeGarbageCollected<V8CompileHints>(this)) {
   DCHECK(!AllPages().Contains(this));
   AllPages().insert(this);
 
@@ -955,6 +956,7 @@ void Page::Trace(Visitor* visitor) const {
   visitor->Trace(next_related_page_);
   visitor->Trace(prev_related_page_);
   visitor->Trace(agent_group_scheduler_);
+  visitor->Trace(v8_compile_hints_);
   Supplementable<Page>::Trace(visitor);
 }
 

third_party/blink/renderer/core/page/page.h

@@ -416,7 +416,7 @@ class CORE_EXPORT Page final : public GarbageCollected<Page>,
     return fenced_frame_mode_;
   }
 
-  V8CompileHints& GetV8CompileHints() { return v8_compile_hints_; }
+  V8CompileHints& GetV8CompileHints() { return *v8_compile_hints_; }
 
  private:
   friend class ScopedPagePauser;
@@ -568,7 +568,7 @@ class CORE_EXPORT Page final : public GarbageCollected<Page>,
 
   WebScopedVirtualTimePauser history_navigation_virtual_time_pauser_;
 
-  V8CompileHints v8_compile_hints_;
+  Member<V8CompileHints> v8_compile_hints_;
 };
 
 extern template class CORE_EXTERN_TEMPLATE_EXPORT Supplement<Page>;

third_party/blink/renderer/platform/scheduler/common/dummy_schedulers.cc

@@ -106,6 +106,7 @@ class DummyFrameScheduler : public FrameScheduler {
   void OnFirstContentfulPaintInMainFrame() override {}
   void OnFirstMeaningfulPaint() override {}
   void OnLoad() override {}
+  void OnMainFrameInteractive() override {}
   bool IsExemptFromBudgetBasedThrottling() const override { return false; }
   std::unique_ptr<blink::mojom::blink::PauseSubresourceLoadingHandle>
   GetPauseSubresourceLoadingHandle() override {

third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl.cc

@@ -831,6 +831,12 @@ void FrameSchedulerImpl::OnFirstContentfulPaintInMainFrame() {
   main_thread_scheduler_->OnMainFramePaint();
 }
 
+void FrameSchedulerImpl::OnMainFrameInteractive() {
+  if (delegate_) {
+    return delegate_->MainFrameInteractive();
+  }
+}
+
 void FrameSchedulerImpl::OnFirstMeaningfulPaint() {
   waiting_for_meaningful_paint_ = false;
 

third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl.h

@@ -121,6 +121,7 @@ class PLATFORM_EXPORT FrameSchedulerImpl : public FrameScheduler,
   void OnFirstContentfulPaintInMainFrame() override;
   void OnFirstMeaningfulPaint() override;
   void OnLoad() override;
+  void OnMainFrameInteractive() override;
   bool IsWaitingForContentfulPaint() const;
   bool IsWaitingForMeaningfulPaint() const;
 

third_party/blink/renderer/platform/scheduler/public/frame_scheduler.h

@@ -44,6 +44,7 @@ class FrameScheduler : public FrameOrWorkerScheduler {
     virtual void OnTaskCompleted(base::TimeTicks start_time,
                                  base::TimeTicks end_time,
                                  base::TimeTicks desired_execution_time) = 0;
+    virtual void MainFrameInteractive() {}
   };
 
   ~FrameScheduler() override = default;
@@ -129,6 +130,10 @@ class FrameScheduler : public FrameOrWorkerScheduler {
   // frame. Only for main frames.
   virtual void OnFirstContentfulPaintInMainFrame() = 0;
 
+  // Tells the scheduler the Frame's Document is interactive. Only for main
+  // frames.
+  virtual void OnMainFrameInteractive() = 0;
+
   // Tells the scheduler that the first meaningful paint has occurred for this
   // frame.
   virtual void OnFirstMeaningfulPaint() = 0;