Throttle resource message requests during user interaction

Resource message requests can be relatively expensive, particularly in
the induced work on the browser IO thread. Currently, there is no bound
on the rate with which such requests are dispatched from the renderer.
This leads to situations where the browser IO thread is flooded with
requests, potentially causing scroll jank and otherwise undesirable
stalls in the browser pipeline.

Introduce a ResourceMessageThrottler which intercepts and defers a given
resource message stream, depending on the state of the RendererScheduler.
When the RendererScheduler indicates that high priority work is
imminent/likely, requests will be throttled according to a configurable
dispatch rate.

Hook this throttling mechanism up to the ResourceDispatcher, limiting
the number of resource message requests/second during user interaction
to 180 (3 per frame at 60 fps) on Android, and 480 on desktop.

See goo.gl/H42AgQ for more design details.

Note: This change originally landed in
https://crrev.com/acfb4199abf841a1577c3968579c43b0232a53b7, but was
reverted due to issues with enforced ThreadChecker validation in
RendererSchedulerImpl. The ThreadChecker validation fix has been split
into a separate patch.

BUG=440037,402136

Review URL: https://codereview.chromium.org/847883002

Cr-Commit-Position: refs/heads/master@{#314767}

content/child/resource_dispatcher.h

@@ -99,6 +99,12 @@ class CONTENT_EXPORT ResourceDispatcher : public IPC::Listener {
       const ResourceMsg_RequestCompleteData& request_complete_data,
       const base::TimeTicks& renderer_completion_time);
 
+  void set_message_sender(IPC::Sender* sender) {
+    DCHECK(sender);
+    DCHECK(pending_requests_.empty());
+    message_sender_ = sender;
+  }
+
   IPC::Sender* message_sender() const { return message_sender_; }
 
   // This does not take ownership of the delegate. It is expected that the

content/content_renderer.gypi

@@ -369,6 +369,8 @@
       'renderer/scheduler/renderer_scheduler_impl.h',
       'renderer/scheduler/renderer_task_queue_selector.cc',
       'renderer/scheduler/renderer_task_queue_selector.h',
+      'renderer/scheduler/resource_dispatch_throttler.cc',
+      'renderer/scheduler/resource_dispatch_throttler.h',
       'renderer/scheduler/single_thread_idle_task_runner.cc',
       'renderer/scheduler/single_thread_idle_task_runner.h',
       'renderer/scheduler/task_queue_manager.cc',

content/content_tests.gypi

@@ -653,6 +653,7 @@
       'renderer/render_widget_unittest.cc',
       'renderer/scheduler/renderer_scheduler_impl_unittest.cc',
       'renderer/scheduler/renderer_task_queue_selector_unittest.cc',
+      'renderer/scheduler/resource_dispatch_throttler_unittest.cc',
       'renderer/scheduler/task_queue_manager_unittest.cc',
       'renderer/screen_orientation/screen_orientation_dispatcher_unittest.cc',
       'renderer/skia_benchmarking_extension_unittest.cc',

content/renderer/render_thread_impl.cc

@@ -105,6 +105,7 @@
 #include "content/renderer/render_view_impl.h"
 #include "content/renderer/renderer_blink_platform_impl.h"
 #include "content/renderer/scheduler/renderer_scheduler.h"
+#include "content/renderer/scheduler/resource_dispatch_throttler.h"
 #include "content/renderer/service_worker/embedded_worker_context_message_filter.h"
 #include "content/renderer/service_worker/embedded_worker_dispatcher.h"
 #include "content/renderer/shared_worker/embedded_shared_worker_stub.h"
@@ -196,6 +197,16 @@ namespace {
 const int64 kInitialIdleHandlerDelayMs = 1000;
 const int64 kLongIdleHandlerDelayMs = 30*1000;
 
+#if defined(OS_ANDROID)
+// On Android, resource messages can each take ~1.5ms to dispatch on the browser
+// IO thread. Limiting the message rate to 3/frame at 60hz ensures that the
+// induced work takes but a fraction (~1/4) of the overall frame budget.
+const int kMaxResourceRequestsPerFlushWhenThrottled = 3;
+#else
+const int kMaxResourceRequestsPerFlushWhenThrottled = 8;
+#endif
+const double kThrottledResourceRequestFlushPeriodS = 1. / 60.;
+
 // Maximum allocation size allowed for image scaling filters that
 // require pre-scaling. Skia will fallback to a filter that doesn't
 // require pre-scaling if the default filter would require an
@@ -475,6 +486,14 @@ void RenderThreadImpl::Init() {
   channel()->SetListenerTaskRunner(renderer_scheduler_->DefaultTaskRunner());
   embedded_worker_dispatcher_.reset(new EmbeddedWorkerDispatcher());
 
+  // Note: This may reorder messages from the ResourceDispatcher with respect to
+  // other subsystems.
+  resource_dispatch_throttler_.reset(new ResourceDispatchThrottler(
+      static_cast<RenderThread*>(this), renderer_scheduler_.get(),
+      base::TimeDelta::FromSecondsD(kThrottledResourceRequestFlushPeriodS),
+      kMaxResourceRequestsPerFlushWhenThrottled));
+  resource_dispatcher()->set_message_sender(resource_dispatch_throttler_.get());
+
   media_stream_center_ = NULL;
 
   db_message_filter_ = new DBMessageFilter();

content/renderer/render_thread_impl.h

@@ -101,6 +101,7 @@ class RenderProcessObserver;
 class RendererBlinkPlatformImpl;
 class RendererDemuxerAndroid;
 class RendererScheduler;
+class ResourceDispatchThrottler;
 class ResourceSchedulingFilter;
 class V8SamplingProfiler;
 class VideoCaptureImplManager;
@@ -476,6 +477,7 @@ class CONTENT_EXPORT RenderThreadImpl
   scoped_ptr<IndexedDBDispatcher> main_thread_indexed_db_dispatcher_;
   scoped_ptr<RendererScheduler> renderer_scheduler_;
   scoped_ptr<RendererBlinkPlatformImpl> blink_platform_impl_;
+  scoped_ptr<ResourceDispatchThrottler> resource_dispatch_throttler_;
   scoped_ptr<EmbeddedWorkerDispatcher> embedded_worker_dispatcher_;
 
   // Used on the render thread and deleted by WebKit at shutdown.

content/renderer/scheduler/null_renderer_scheduler.cc

@@ -78,6 +78,10 @@ void NullRendererScheduler::DidReceiveInputEventOnCompositorThread(
 void NullRendererScheduler::DidAnimateForInputOnCompositorThread() {
 }
 
+bool NullRendererScheduler::IsHighPriorityWorkAnticipated() {
+  return false;
+}
+
 bool NullRendererScheduler::ShouldYieldForHighPriorityWork() {
   return false;
 }

content/renderer/scheduler/null_renderer_scheduler.h

@@ -24,6 +24,7 @@ class NullRendererScheduler : public RendererScheduler {
   void DidReceiveInputEventOnCompositorThread(
       blink::WebInputEvent::Type type) override;
   void DidAnimateForInputOnCompositorThread() override;
+  bool IsHighPriorityWorkAnticipated() override;
   bool ShouldYieldForHighPriorityWork() override;
   void Shutdown() override;
 

content/renderer/scheduler/renderer_scheduler.h

@@ -53,8 +53,16 @@ class CONTENT_EXPORT RendererScheduler {
   // a fling). Called by the compositor (impl) thread.
   virtual void DidAnimateForInputOnCompositorThread() = 0;
 
+  // Returns true if the scheduler has reason to believe that high priority work
+  // may soon arrive on the main thread, e.g., if gesture events were observed
+  // recently.
+  // Must be called from the main thread.
+  virtual bool IsHighPriorityWorkAnticipated() = 0;
+
   // Returns true if there is high priority work pending on the main thread
-  // and the caller should yield to let the scheduler service that work.
+  // and the caller should yield to let the scheduler service that work. Note
+  // that this is a stricter condition than |IsHighPriorityWorkAnticipated|,
+  // restricted to the case where real work is pending.
   // Must be called from the main thread.
   virtual bool ShouldYieldForHighPriorityWork() = 0;
 

content/renderer/scheduler/renderer_scheduler_impl.cc

@@ -147,6 +147,15 @@ void RendererSchedulerImpl::UpdateForInputEvent() {
   last_input_time_ = Now();
 }
 
+bool RendererSchedulerImpl::IsHighPriorityWorkAnticipated() {
+  main_thread_checker_.CalledOnValidThread();
+  if (!task_queue_manager_)
+    return false;
+
+  MaybeUpdatePolicy();
+  return SchedulerPolicy() == COMPOSITOR_PRIORITY_POLICY;
+}
+
 bool RendererSchedulerImpl::ShouldYieldForHighPriorityWork() {
   main_thread_checker_.CalledOnValidThread();
   if (!task_queue_manager_)
@@ -157,6 +166,8 @@ bool RendererSchedulerImpl::ShouldYieldForHighPriorityWork() {
   // work outstanding. Note: even though the control queue is higher priority
   // we don't yield for it since these tasks are not user-provided work and they
   // are only intended to run before the next task, not interrupt the tasks.
+  // Note: This function could conceivably be implemented in terms of
+  // |IsHighPriorityWorkAnticipated|, but for clarity is not.
   return SchedulerPolicy() == COMPOSITOR_PRIORITY_POLICY &&
          !task_queue_manager_->IsQueueEmpty(COMPOSITOR_TASK_QUEUE);
 }

content/renderer/scheduler/renderer_scheduler_impl.h

@@ -40,6 +40,7 @@ class CONTENT_EXPORT RendererSchedulerImpl : public RendererScheduler {
   void DidReceiveInputEventOnCompositorThread(
       blink::WebInputEvent::Type type) override;
   void DidAnimateForInputOnCompositorThread() override;
+  bool IsHighPriorityWorkAnticipated() override;
   bool ShouldYieldForHighPriorityWork() override;
   void Shutdown() override;
 

content/renderer/scheduler/renderer_scheduler_impl_unittest.cc

@@ -36,6 +36,11 @@ class RendererSchedulerImplTest : public testing::Test {
   }
 
  protected:
+  static base::TimeDelta compositor_priority_after_touch_duration() {
+    return base::TimeDelta::FromMilliseconds(
+        RendererSchedulerImpl::kCompositorPriorityAfterTouchMillis);
+  }
+
   scoped_refptr<cc::TestNowSource> clock_;
   scoped_refptr<cc::OrderedSimpleTaskRunner> mock_task_runner_;
 
@@ -132,6 +137,18 @@ void PostingYieldingTestTask(
   *should_yield_after = scheduler->ShouldYieldForHighPriorityWork();
 }
 
+void AnticipationTestTask(RendererSchedulerImpl* scheduler,
+                          bool simulate_input,
+                          bool* is_anticipated_before,
+                          bool* is_anticipated_after) {
+  *is_anticipated_before = scheduler->IsHighPriorityWorkAnticipated();
+  if (simulate_input) {
+    scheduler->DidReceiveInputEventOnCompositorThread(
+        blink::WebInputEvent::GestureFlingStart);
+  }
+  *is_anticipated_after = scheduler->IsHighPriorityWorkAnticipated();
+}
+
 TEST_F(RendererSchedulerImplTest, TestPostDefaultTask) {
   int result = 0;
   default_task_runner_->PostTask(FROM_HERE,
@@ -499,6 +516,45 @@ TEST_F(RendererSchedulerImplTest, TestCompositorPolicyEnds) {
                                    std::string("D2"), std::string("C2")));
 }
 
+TEST_F(RendererSchedulerImplTest, TestIsHighPriorityWorkAnticipated) {
+  bool is_anticipated_before = false;
+  bool is_anticipated_after = false;
+
+  bool simulate_input = false;
+  default_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&AnticipationTestTask, scheduler_.get(), simulate_input,
+                 &is_anticipated_before, &is_anticipated_after));
+  RunUntilIdle();
+  // In its default state, without input receipt, the scheduler should indicate
+  // that no high-priority is anticipated.
+  EXPECT_FALSE(is_anticipated_before);
+  EXPECT_FALSE(is_anticipated_after);
+
+  simulate_input = true;
+  default_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&AnticipationTestTask, scheduler_.get(), simulate_input,
+                 &is_anticipated_before, &is_anticipated_after));
+  RunUntilIdle();
+  // When input is received, the scheduler should indicate that high-priority
+  // work is anticipated.
+  EXPECT_FALSE(is_anticipated_before);
+  EXPECT_TRUE(is_anticipated_after);
+
+  clock_->AdvanceNow(compositor_priority_after_touch_duration() * 2);
+  simulate_input = false;
+  default_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&AnticipationTestTask, scheduler_.get(), simulate_input,
+                 &is_anticipated_before, &is_anticipated_after));
+  RunUntilIdle();
+  // Without additional input, the scheduler should indicate that high-priority
+  // work is no longer anticipated.
+  EXPECT_FALSE(is_anticipated_before);
+  EXPECT_FALSE(is_anticipated_after);
+}
+
 TEST_F(RendererSchedulerImplTest, TestShouldYield) {
   bool should_yield_before = false;
   bool should_yield_after = false;

content/renderer/scheduler/resource_dispatch_throttler.cc

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/renderer/scheduler/resource_dispatch_throttler.h"
+
+#include "base/auto_reset.h"
+#include "base/trace_event/trace_event.h"
+#include "content/common/resource_messages.h"
+#include "content/renderer/scheduler/renderer_scheduler.h"
+#include "ipc/ipc_message_macros.h"
+
+namespace content {
+namespace {
+
+bool IsResourceRequest(const IPC::Message& msg) {
+  return msg.type() == ResourceHostMsg_RequestResource::ID;
+}
+
+}  // namespace
+
+ResourceDispatchThrottler::ResourceDispatchThrottler(
+    IPC::Sender* proxied_sender,
+    RendererScheduler* scheduler,
+    base::TimeDelta flush_period,
+    uint32 max_requests_per_flush)
+    : proxied_sender_(proxied_sender),
+      scheduler_(scheduler),
+      flush_period_(flush_period),
+      max_requests_per_flush_(max_requests_per_flush),
+      flush_timer_(
+          FROM_HERE,
+          flush_period_,
+          base::Bind(&ResourceDispatchThrottler::Flush, base::Unretained(this)),
+          false /* is_repeating */),
+      sent_requests_since_last_flush_(0) {
+  DCHECK(proxied_sender);
+  DCHECK(scheduler);
+  DCHECK_NE(flush_period_, base::TimeDelta());
+  DCHECK(max_requests_per_flush_);
+  flush_timer_.SetTaskRunner(scheduler->LoadingTaskRunner());
+}
+
+ResourceDispatchThrottler::~ResourceDispatchThrottler() {
+  FlushAll();
+}
+
+bool ResourceDispatchThrottler::Send(IPC::Message* msg) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (msg->is_sync()) {
+    // Flush any pending requests, preserving dispatch order between async and
+    // sync requests.
+    FlushAll();
+    return ForwardMessage(msg);
+  }
+
+  // Always defer message forwarding if there are pending messages, ensuring
+  // message dispatch ordering consistency.
+  if (!throttled_messages_.empty()) {
+    TRACE_EVENT_INSTANT0("loader", "ResourceDispatchThrottler::ThrottleMessage",
+                         TRACE_EVENT_SCOPE_THREAD);
+    throttled_messages_.push_back(msg);
+    return true;
+  }
+
+  if (!IsResourceRequest(*msg))
+    return ForwardMessage(msg);
+
+  if (!scheduler_->IsHighPriorityWorkAnticipated())
+    return ForwardMessage(msg);
+
+  if (Now() > (last_sent_request_time_ + flush_period_)) {
+    // If sufficient time has passed since the previous send, we can effectively
+    // mark the pipeline as flushed.
+    sent_requests_since_last_flush_ = 0;
+    return ForwardMessage(msg);
+  }
+
+  if (sent_requests_since_last_flush_ < max_requests_per_flush_)
+    return ForwardMessage(msg);
+
+  TRACE_EVENT_INSTANT0("loader", "ResourceDispatchThrottler::ThrottleRequest",
+                       TRACE_EVENT_SCOPE_THREAD);
+  throttled_messages_.push_back(msg);
+  ScheduleFlush();
+  return true;
+}
+
+base::TimeTicks ResourceDispatchThrottler::Now() const {
+  return base::TimeTicks::Now();
+}
+
+void ResourceDispatchThrottler::ScheduleFlush() {
+  DCHECK(!flush_timer_.IsRunning());
+  flush_timer_.Reset();
+}
+
+void ResourceDispatchThrottler::Flush() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  TRACE_EVENT1("loader", "ResourceDispatchThrottler::Flush",
+               "total_throttled_messages", throttled_messages_.size());
+  sent_requests_since_last_flush_ = 0;
+
+  // If high-priority work is no longer anticipated, dispatch can be safely
+  // accelerated. Avoid completely flushing in such case in the event that
+  // a large number of requests have been throttled.
+  uint32 max_requests = scheduler_->IsHighPriorityWorkAnticipated()
+                            ? max_requests_per_flush_
+                            : max_requests_per_flush_ * 2;
+
+  while (!throttled_messages_.empty() &&
+         (sent_requests_since_last_flush_ < max_requests ||
+          !IsResourceRequest(*throttled_messages_.front()))) {
+    IPC::Message* msg = throttled_messages_.front();
+    throttled_messages_.pop_front();
+    ForwardMessage(msg);
+  }
+
+  if (!throttled_messages_.empty())
+    ScheduleFlush();
+}
+
+void ResourceDispatchThrottler::FlushAll() {
+  if (throttled_messages_.empty())
+    return;
+
+  TRACE_EVENT1("loader", "ResourceDispatchThrottler::FlushAll",
+               "total_throttled_messages", throttled_messages_.size());
+  std::deque<IPC::Message*> throttled_messages;
+  throttled_messages.swap(throttled_messages_);
+  for (auto& message : throttled_messages)
+    ForwardMessage(message);
+  // There shouldn't be re-entrancy issues when forwarding an IPC, but validate
+  // as a safeguard.
+  DCHECK(throttled_messages_.empty());
+}
+
+bool ResourceDispatchThrottler::ForwardMessage(IPC::Message* msg) {
+  if (IsResourceRequest(*msg)) {
+    last_sent_request_time_ = Now();
+    ++sent_requests_since_last_flush_;
+  }
+  return proxied_sender_->Send(msg);
+}
+
+}  // namespace content

content/renderer/scheduler/resource_dispatch_throttler.h

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CONTENT_RENDERER_SCHEDULER_RESOURCE_DISPATCH_THROTTLER_H_
+#define CONTENT_RENDERER_SCHEDULER_RESOURCE_DISPATCH_THROTTLER_H_
+
+#include <deque>
+
+#include "base/threading/thread_checker.h"
+#include "base/time/time.h"
+#include "base/timer/timer.h"
+#include "content/common/content_export.h"
+#include "ipc/ipc_sender.h"
+
+namespace content {
+
+class RendererScheduler;
+
+// Utility class for throttling a stream of resource requests targetted to a
+// specific IPC sender. The throttling itself is very basic:
+//  * When there is no high-priority work imminent to the main thread, as
+//    indicated by the RendererScheduler, throttling is disabled.
+//  * When >= N requests have been sent in a given time window, requests are
+//    throttled. A timer periodically flushes a portion of the queued requests
+//    until all such requests have been flushed.
+// TODO(jdduke): Remove this class after resource requests become sufficiently
+// cheap on the IO thread, crbug.com/440037.
+class CONTENT_EXPORT ResourceDispatchThrottler : public IPC::Sender {
+ public:
+  // |flush_period| and |max_requests_per_flush| must be strictly positive
+  // in duration/value.
+  ResourceDispatchThrottler(IPC::Sender* proxied_sender,
+                            RendererScheduler* scheduler,
+                            base::TimeDelta flush_period,
+                            uint32 max_requests_per_flush);
+  ~ResourceDispatchThrottler() override;
+
+  // IPC::Sender implementation:
+  bool Send(IPC::Message* msg) override;
+
+ private:
+  friend class ResourceDispatchThrottlerForTest;
+
+  // Virtual for testing.
+  virtual base::TimeTicks Now() const;
+  virtual void ScheduleFlush();
+
+  void Flush();
+  void FlushAll();
+  bool ForwardMessage(IPC::Message* msg);
+
+  base::ThreadChecker thread_checker_;
+
+  IPC::Sender* const proxied_sender_;
+  RendererScheduler* const scheduler_;
+  const base::TimeDelta flush_period_;
+  const uint32 max_requests_per_flush_;
+
+  base::Timer flush_timer_;
+  base::TimeTicks last_sent_request_time_;
+  uint32 sent_requests_since_last_flush_;
+  std::deque<IPC::Message*> throttled_messages_;
+
+  DISALLOW_COPY_AND_ASSIGN(ResourceDispatchThrottler);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_RENDERER_SCHEDULER_RESOURCE_DISPATCH_THROTTLER_H_

content/test/fake_renderer_scheduler.cc

@@ -45,6 +45,10 @@ void FakeRendererScheduler::DidReceiveInputEventOnCompositorThread(
 void FakeRendererScheduler::DidAnimateForInputOnCompositorThread() {
 }
 
+bool FakeRendererScheduler::IsHighPriorityWorkAnticipated() {
+  return false;
+}
+
 bool FakeRendererScheduler::ShouldYieldForHighPriorityWork() {
   return false;
 }

content/test/fake_renderer_scheduler.h

@@ -24,6 +24,7 @@ class FakeRendererScheduler : public RendererScheduler {
   void DidReceiveInputEventOnCompositorThread(
       blink::WebInputEvent::Type type) override;
   void DidAnimateForInputOnCompositorThread() override;
+  bool IsHighPriorityWorkAnticipated() override;
   bool ShouldYieldForHighPriorityWork() override;
   void Shutdown() override;