Add MemoryCoordinatorImpl

content/browser/memory/memory_coordinator_impl.cc

Index: content/browser/memory/memory_coordinator_impl.cc
diff --git a/content/browser/memory/memory_coordinator_impl.cc b/content/browser/memory/memory_coordinator_impl.cc
new file mode 100644
index 0000000000000000000000000000000000000000..9528e730906575f1d9796883bb721f700ed2268d
--- /dev/null
+++ b/content/browser/memory/memory_coordinator_impl.cc
@@ -0,0 +1,175 @@
+// Copyright 2016 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/browser/memory/memory_coordinator_impl.h"
+
+#include "base/threading/thread_task_runner_handle.h"
+#include "content/browser/memory/memory_monitor.h"
+#include "content/public/common/content_features.h"
+
+namespace content {
+
+namespace {
+
+#if defined(OS_ANDROID)
+static const int kDefaultPredictedRendererSizeMB = 40;

[chrisha, 2016/10/10 23:53:01]

Can you comment where these come from (ie, the XXth percentile of the
appropriate UMA stat). Also, it would be good to define appropriate values for
Linux, ChromeOS, Windows, MacOS and Android while you're at it.

[bashi, 2016/10/11 02:07:33]

Done. I was thinking that that's kind of information is a bit sensitive, but it
seems not. Memory.Renderer.Large2 tells me that MacOS, Linux and ChromeOS have a
similar median.

+#else
+static const int kDefaultPredictedRendererSizeMB = 80;
+#endif
+
+mojom::MemoryState ToMojomMemoryState(base::MemoryState state) {
+  switch (state) {
+    case base::MemoryState::UNKNOWN:
+      return mojom::MemoryState::UNKNOWN;
+    case base::MemoryState::NORMAL:
+      return mojom::MemoryState::NORMAL;
+    case base::MemoryState::THROTTLED:
+      return mojom::MemoryState::THROTTLED;
+    case base::MemoryState::SUSPENDED:
+      return mojom::MemoryState::SUSPENDED;
+    default:
+      NOTREACHED();
+      return mojom::MemoryState::UNKNOWN;
+  }
+}
+
+}  // namespace
+
+// SingletonTraits for MemoryCoordinator. Returns MemoryCoordinatorImpl
+// as an actual instance.
+struct MemoryCoordinatorSingletonTraits
+    : public base::LeakySingletonTraits<MemoryCoordinator> {
+  static MemoryCoordinator* New() {
+    return new MemoryCoordinatorImpl(base::ThreadTaskRunnerHandle::Get(),
+                                     CreateMemoryMonitor());
+  }
+};
+
+// static
+MemoryCoordinator* MemoryCoordinator::GetInstance() {
+  if (!base::FeatureList::IsEnabled(features::kMemoryCoordinator))
+    return nullptr;
+  return base::Singleton<MemoryCoordinator,
+                         MemoryCoordinatorSingletonTraits>::get();
+}
+
+MemoryCoordinatorImpl::MemoryCoordinatorImpl(
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+    std::unique_ptr<MemoryMonitor> memory_monitor)
+    : task_runner_(task_runner),
+      memory_monitor_(std::move(memory_monitor)),
+      weak_ptr_factory_(this) {
+  DCHECK(memory_monitor_.get());
+  update_state_callback_ = base::Bind(&MemoryCoordinatorImpl::UpdateState,
+                                      weak_ptr_factory_.GetWeakPtr());
+
+  // Set initial parameters for calculating the global state.
+  predicted_renderer_size_ = kDefaultPredictedRendererSizeMB;
+  new_renderers_until_throttled_ = 4;
+  new_renderers_until_suspended_ = 2;
+  new_renderers_back_to_normal_ = 5;
+  new_renderers_back_to_throttled_ = 3;

[chrisha, 2016/10/10 23:53:02]

I'd prefer to have constants defined in an anonymous namespace above, and then
referenced here:

const int kDefaultNewRenderersUntilThrottled = 4;
etc...

[bashi, 2016/10/11 02:07:33]

Done.

+  minimum_transition_period_ = base::TimeDelta::FromSeconds(30);
+  monitoring_interval_ = base::TimeDelta::FromSeconds(5);

[chrisha, 2016/10/10 23:53:02]

Ditto.

[bashi, 2016/10/11 02:07:33]

Done.

+}
+
+MemoryCoordinatorImpl::~MemoryCoordinatorImpl() {}
+
+void MemoryCoordinatorImpl::Start() {
+  DCHECK(CalledOnValidThread());
+  DCHECK(last_state_change_.is_null());
+  DCHECK(ValidateParameters());
+  ScheduleUpdateState(base::TimeDelta());
+}
+
+void MemoryCoordinatorImpl::OnChildAdded(int render_process_id) {
+  // Populate the global state as an initial state of a newly created process.
+  SetMemoryState(render_process_id, ToMojomMemoryState(current_state_));

[chrisha, 2016/10/10 23:53:02]

Either SetMemoryState needs to be defensive and only allow valid transitions, or
we need to check that the state is valid for the given process. For example, if
this is a new foreground tab, we shouldn't ever be trying to suspend or throttle
it.

[bashi, 2016/10/11 02:07:33]

I've added checks in SetMemoryState() to prevent invalid transitions in
https://codereview.chromium.org/2399293002/

Should we avoid throttling a new foreground tab? I think throttling a foreground
tab from the beginning makes sense.

[chrisha, 2016/10/14 20:06:05]

I'm not sure what the right answer is. Under extreme pressure we should allow
throttling a foreground page, but I'd say we should only do that after having
purged + suspended all pages, and maybe even only after discarding pages and
still being under pressure?

We want to make sure the foreground page always has the best experience
possible. For now I'd simply say not to throttle them, and we can explore that
as a refinement later.

[bashi, 2016/10/16 23:51:22]

I see. Added CanThrottleRenderer() which checks whether a tab is backgrounded.

+}
+
+base::MemoryState MemoryCoordinatorImpl::CalculateNextState() {
+  using MemoryState = base::MemoryState;
+
+  int available = memory_monitor_->GetFreeMemoryUntilCriticalMB();
+  if (available <= 0)
+    return MemoryState::SUSPENDED;
+
+  int new_renderers_until_critical = available / predicted_renderer_size_;

[chrisha, 2016/10/10 23:53:02]

expected_renderer_size_ is a better name maybe?

(We're not doing active work to try to make a good prediction... but that would
be a potential refinement.)

[bashi, 2016/10/11 02:07:33]

Yeah, changed to expected_renderer_count.

+
+  switch (current_state_) {
+    case MemoryState::NORMAL:
+      if (new_renderers_until_critical <= new_renderers_until_suspended_)
+        return MemoryState::SUSPENDED;
+      if (new_renderers_until_critical <= new_renderers_until_throttled_)
+        return MemoryState::THROTTLED;
+      return MemoryState::NORMAL;
+    case MemoryState::THROTTLED:
+      if (new_renderers_until_critical <= new_renderers_until_suspended_)
+        return MemoryState::SUSPENDED;
+      if (new_renderers_until_critical >= new_renderers_back_to_normal_)
+        return MemoryState::NORMAL;
+      return MemoryState::THROTTLED;
+    case MemoryState::SUSPENDED:
+      if (new_renderers_until_critical >= new_renderers_back_to_normal_)
+        return MemoryState::NORMAL;
+      if (new_renderers_until_critical >= new_renderers_back_to_throttled_)
+        return MemoryState::THROTTLED;
+      return MemoryState::SUSPENDED;
+    case MemoryState::UNKNOWN:
+      // Fall through
+    default:
+      NOTREACHED();
+      return MemoryState::UNKNOWN;
+  }
+}
+
+void MemoryCoordinatorImpl::UpdateState() {
+  base::TimeTicks now = base::TimeTicks::Now();
+  MemoryState prev_state = current_state_;
+  MemoryState next_state = CalculateNextState();
+
+  if (last_state_change_.is_null() || current_state_ != next_state) {
+    current_state_ = next_state;
+    last_state_change_ = now;
+  }
+
+  if (next_state != prev_state) {
+    NotifyStateToClients();
+    NotifyStateToChildren();
+    ScheduleUpdateState(minimum_transition_period_);
+  } else {
+    ScheduleUpdateState(monitoring_interval_);
+  }
+}
+
+void MemoryCoordinatorImpl::NotifyStateToClients() {
+  // SUSPENDED state may not make sense to the browser process. Use THROTTLED
+  // instead when the global state is SUSPENDED.
+  // TODO(bashi): Maybe worth considering another state for the browser.
+  auto state = current_state_ == MemoryState::SUSPENDED ? MemoryState::THROTTLED
+                                                        : current_state_;
+  base::MemoryCoordinatorClientRegistry::GetInstance()->Notify(state);
+}
+
+void MemoryCoordinatorImpl::NotifyStateToChildren() {
+  // TODO(bashi): Don't send SUSPENDED state to the foreground renderer.
+  // TODO(bashi): Consider how to deal with background renderers which can't
+  // be suspended.

[chrisha, 2016/10/10 23:53:02]

These TODOs need to happen before this can land, IMO.

Have a CanThrottle and a CanSuspend in the delegate? Or a more general
CanBeInState?
Then iterate over the renderers and only transition those that can actually be
in the new state?

(In other words, this CL should land after the delegate CL.)

[bashi, 2016/10/11 02:07:33]

Yes. I'll submit this after the delegate is landed.
Hmm, I'd prefer to put logic in content/ as much as possible. IIUC, memory
coordinator can determine if a renderer can be throttled without information in
chrome/browser so I think CanSuspend() is enough at this point. How about adding
CanBeInState() in MemoryCoordinatorImpl?

+  auto mojo_state = ToMojomMemoryState(current_state_);
+  for (auto& iter : children())
+    SetMemoryState(iter.first, mojo_state);
+}
+
+void MemoryCoordinatorImpl::ScheduleUpdateState(base::TimeDelta delta) {
+  task_runner_->PostDelayedTask(FROM_HERE, update_state_callback_, delta);
+}
+
+bool MemoryCoordinatorImpl::ValidateParameters() {
+  return (new_renderers_until_throttled_ > new_renderers_until_suspended_) &&
+      (new_renderers_back_to_normal_ > new_renderers_back_to_throttled_) &&
+      (new_renderers_back_to_normal_ > new_renderers_until_throttled_) &&
+      (new_renderers_back_to_throttled_ > new_renderers_until_suspended_);
+}
+
+}  // namespace content