Skip to content

Commit

Permalink
Reland "Compositing-based frame throttling."
Browse files Browse the repository at this point in the history
This relands commit d48bbee.

A number of Asan tests failed on the reverted commit due to the use of
ScopedMultiSourceObservation without properly removing the sources
(WindowTreeHost*) when they go out of scope at places other than when
the hosts are not deleted from WindowHostManager. This commit simplifies
the observer pattern used on FrameThrottlingController as
aura::WindowTreeHostObserver.

Bug: 1143872,1179740,1179695
Change-Id: Ifaf903af7c824c5609ada25013bd4977e5086910
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2705310
Commit-Queue: Jun Liu <yjliu@chromium.org>
Auto-Submit: Jun Liu <yjliu@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#857938}
  • Loading branch information
yjliu authored and Chromium LUCI CQ committed Feb 26, 2021
1 parent 229f9bc commit a98c7e4
Show file tree
Hide file tree
Showing 31 changed files with 539 additions and 13 deletions.
2 changes: 2 additions & 0 deletions ash/display/window_tree_host_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "ash/display/cursor_window_controller.h"
#include "ash/display/mirror_window_controller.h"
#include "ash/display/root_window_transformers.h"
#include "ash/frame_throttler/frame_throttling_controller.h"
#include "ash/host/ash_window_tree_host.h"
#include "ash/host/ash_window_tree_host_init_params.h"
#include "ash/host/root_window_transformer.h"
Expand Down Expand Up @@ -868,6 +869,7 @@ AshWindowTreeHost* WindowTreeHostManager::AddWindowTreeHostForDisplay(
AshWindowTreeHost* ash_host =
AshWindowTreeHost::Create(params_with_bounds).release();
aura::WindowTreeHost* host = ash_host->AsWindowTreeHost();
Shell::Get()->frame_throttling_controller()->OnWindowTreeHostCreated(host);
DCHECK(!host->has_input_method());
if (!input_method_) { // Singleton input method instance for Ash.
input_method_ = ui::CreateInputMethod(this, host->GetAcceleratedWidget());
Expand Down
57 changes: 57 additions & 0 deletions ash/frame_throttler/frame_throttling_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "components/viz/host/host_frame_sink_manager.h"
#include "ui/aura/client/aura_constants.h"
#include "ui/aura/window.h"
#include "ui/aura/window_tree_host.h"

namespace ash {

Expand All @@ -42,6 +43,28 @@ void CollectBrowserFrameSinkIds(const std::vector<aura::Window*>& windows,
}
}

// Recursively walks through all descendents of |window| and collects those
// belonging to a browser and with their frame sink ids being a member of |ids|.
// |inside_browser| indicates if the |window| already belongs to a browser.
// |browser_ids| is the output containing the result frame sinks ids.
void CollectBrowserFrameSinkIdsInWindow(
const aura::Window* window,
bool inside_browser,
const base::flat_set<viz::FrameSinkId>& ids,
base::flat_set<viz::FrameSinkId>* browser_ids) {
if (inside_browser || ash::AppType::BROWSER ==
static_cast<ash::AppType>(
window->GetProperty(aura::client::kAppType))) {
const auto& id = window->GetFrameSinkId();
if (id.is_valid() && ids.contains(id))
browser_ids->insert(id);
inside_browser = true;
}

for (auto* child : window->children())
CollectBrowserFrameSinkIdsInWindow(child, inside_browser, ids, browser_ids);
}

} // namespace

FrameThrottlingController::FrameThrottlingController(
Expand Down Expand Up @@ -157,6 +180,40 @@ void FrameThrottlingController::EndThrottling() {
windows_throttled_ = false;
}

void FrameThrottlingController::OnCompositingFrameSinksToThrottleUpdated(
const aura::WindowTreeHost* window_tree_host,
const base::flat_set<viz::FrameSinkId>& ids) {
base::flat_set<viz::FrameSinkId>& browser_ids =
host_to_ids_map_[window_tree_host];
browser_ids.clear();
CollectBrowserFrameSinkIdsInWindow(window_tree_host->window(), false, ids,
&browser_ids);
UpdateThrottling();
}

void FrameThrottlingController::OnWindowDestroying(aura::Window* window) {
DCHECK(window->IsRootWindow());
host_to_ids_map_.erase(window->GetHost());
UpdateThrottling();
window->RemoveObserver(this);
}

void FrameThrottlingController::OnWindowTreeHostCreated(
aura::WindowTreeHost* host) {
host->AddObserver(this);
host->window()->AddObserver(this);
}

void FrameThrottlingController::UpdateThrottling() {
std::vector<viz::FrameSinkId> ids_to_throttle;
for (const auto& pair : host_to_ids_map_) {
ids_to_throttle.insert(ids_to_throttle.end(), pair.second.begin(),
pair.second.end());
}
context_factory_->GetHostFrameSinkManager()->Throttle(
ids_to_throttle, base::TimeDelta::FromHz(throttled_fps_));
}

void FrameThrottlingController::AddObserver(FrameThrottlingObserver* observer) {
observers_.AddObserver(observer);
}
Expand Down
31 changes: 29 additions & 2 deletions ash/frame_throttler/frame_throttling_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,17 @@

#include "ash/ash_export.h"
#include "ash/frame_throttler/frame_throttling_observer.h"
#include "base/containers/flat_map.h"
#include "base/containers/flat_set.h"
#include "base/macros.h"
#include "base/observer_list.h"
#include "base/observer_list_types.h"
#include "components/viz/common/surfaces/frame_sink_id.h"
#include "ui/aura/window_observer.h"
#include "ui/aura/window_tree_host_observer.h"

namespace aura {
class WindowTreeHost;
class Window;
}

Expand All @@ -27,13 +32,25 @@ namespace ash {

constexpr uint8_t kDefaultThrottleFps = 20;

class ASH_EXPORT FrameThrottlingController {
class ASH_EXPORT FrameThrottlingController
: public aura::WindowTreeHostObserver,
public aura::WindowObserver {
public:
explicit FrameThrottlingController(ui::ContextFactory* context_factory);
FrameThrottlingController(const FrameThrottlingController&) = delete;
FrameThrottlingController& operator=(const FrameThrottlingController&) =
delete;
~FrameThrottlingController();
~FrameThrottlingController() final;

// ui::WindowTreeHostObserver overrides
void OnCompositingFrameSinksToThrottleUpdated(
const aura::WindowTreeHost* host,
const base::flat_set<viz::FrameSinkId>& ids) override;

// ui::WindowObserver overrides
void OnWindowDestroying(aura::Window* window) override;

void OnWindowTreeHostCreated(aura::WindowTreeHost* host);

// Starts to throttle the framerate of |windows|.
void StartThrottling(const std::vector<aura::Window*>& windows);
Expand All @@ -55,10 +72,20 @@ class ASH_EXPORT FrameThrottlingController {
void EndThrottlingFrameSinks();
void EndThrottlingArc();

void UpdateThrottling();

ui::ContextFactory* context_factory_ = nullptr;
base::ObserverList<FrameThrottlingObserver> observers_;
base::ObserverList<FrameThrottlingObserver> arc_observers_;

// Maps aura::WindowTreeHost* to a set of FrameSinkIds to be throttled.
using WindowTreeHostMap = base::flat_map<const aura::WindowTreeHost*,
base::flat_set<viz::FrameSinkId>>;
// Compositing-based throttling updates the set of FrameSinkIds per tree and
// this map keeps each aura::WindowTreeHost* to the most recent updated
// FrameSinkIds.
WindowTreeHostMap host_to_ids_map_;

// The fps used for throttling.
uint8_t throttled_fps_ = kDefaultThrottleFps;
bool windows_throttled_ = false;
Expand Down
5 changes: 3 additions & 2 deletions ash/shell.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1025,6 +1025,9 @@ void Shell::Init(

screen_position_controller_ = std::make_unique<ScreenPositionController>();

frame_throttling_controller_ =
std::make_unique<FrameThrottlingController>(context_factory);

window_tree_host_manager_->Start();
AshWindowTreeHostInitParams ash_init_params;
window_tree_host_manager_->CreatePrimaryHost(ash_init_params);
Expand Down Expand Up @@ -1250,8 +1253,6 @@ void Shell::Init(
sms_observer_.reset(new SmsObserver());
snap_controller_ = std::make_unique<SnapControllerImpl>();
key_accessibility_enabler_ = std::make_unique<KeyAccessibilityEnabler>();
frame_throttling_controller_ =
std::make_unique<FrameThrottlingController>(context_factory);

// Create UserSettingsEventLogger after |system_tray_model_| and
// |video_detector_| which it observes.
Expand Down
3 changes: 3 additions & 0 deletions cc/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,8 @@ cc_component("cc") {
"trees/target_property.h",
"trees/task_runner_provider.cc",
"trees/task_runner_provider.h",
"trees/throttle_decider.cc",
"trees/throttle_decider.h",
"trees/transform_node.cc",
"trees/transform_node.h",
"trees/tree_synchronizer.cc",
Expand Down Expand Up @@ -792,6 +794,7 @@ cc_test("cc_unittests") {
"trees/property_tree_builder_unittest.cc",
"trees/property_tree_unittest.cc",
"trees/swap_promise_manager_unittest.cc",
"trees/throttle_decider_unittest.cc",
"trees/tree_synchronizer_unittest.cc",
"trees/ukm_manager_unittest.cc",

Expand Down
2 changes: 2 additions & 0 deletions cc/test/fake_layer_tree_host_impl_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ class FakeLayerTreeHostImplClient : public LayerTreeHostImplClient {
base::TimeDelta first_scroll_delay,
base::TimeTicks first_scroll_timestamp) override {}
bool IsInSynchronousComposite() const override;
void FrameSinksToThrottleUpdated(
const base::flat_set<viz::FrameSinkId>& ids) override {}

void reset_did_request_impl_side_invalidation() {
did_request_impl_side_invalidation_ = false;
Expand Down
10 changes: 10 additions & 0 deletions cc/trees/layer_tree_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1230,6 +1230,8 @@ DrawResult LayerTreeHostImpl::CalculateRenderPasses(FrameData* frame) {
// Advance our de-jelly state. This is a no-op if de-jelly is not active.
de_jelly_state_.AdvanceFrame(active_tree_.get());

if (settings_.enable_compositing_based_throttling)
throttle_decider_.Prepare();
for (EffectTreeLayerListIterator it(active_tree());
it.state() != EffectTreeLayerListIterator::State::END; ++it) {
auto target_render_pass_id = it.target_render_surface()->render_pass_id();
Expand All @@ -1247,6 +1249,8 @@ DrawResult LayerTreeHostImpl::CalculateRenderPasses(FrameData* frame) {
render_surface->EffectTreeIndex(),
&target_render_pass->copy_requests);
}
if (settings_.enable_compositing_based_throttling && target_render_pass)
throttle_decider_.ProcessRenderPass(*target_render_pass);
} else if (it.state() ==
EffectTreeLayerListIterator::State::CONTRIBUTING_SURFACE) {
RenderSurfaceImpl* render_surface = it.current_render_surface();
Expand Down Expand Up @@ -2414,6 +2418,12 @@ bool LayerTreeHostImpl::DrawLayers(FrameData* frame) {
devtools_instrumentation::DidDrawFrame(id_);
benchmark_instrumentation::IssueImplThreadRenderingStatsEvent(
rendering_stats_instrumentation_->TakeImplThreadRenderingStats());

if (settings_.enable_compositing_based_throttling &&
throttle_decider_.HasThrottlingChanged()) {
client_->FrameSinksToThrottleUpdated(throttle_decider_.ids());
}

return true;
}

Expand Down
8 changes: 8 additions & 0 deletions cc/trees/layer_tree_host_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
#include "cc/trees/presentation_time_callback_buffer.h"
#include "cc/trees/render_frame_metadata.h"
#include "cc/trees/task_runner_provider.h"
#include "cc/trees/throttle_decider.h"
#include "cc/trees/ukm_manager.h"
#include "components/viz/client/client_resource_provider.h"
#include "components/viz/common/frame_sinks/begin_frame_args.h"
Expand Down Expand Up @@ -190,6 +191,9 @@ class LayerTreeHostImplClient {
// code as a result.
virtual bool IsInSynchronousComposite() const = 0;

virtual void FrameSinksToThrottleUpdated(
const base::flat_set<viz::FrameSinkId>& ids) = 0;

protected:
virtual ~LayerTreeHostImplClient() = default;
};
Expand Down Expand Up @@ -1231,6 +1235,10 @@ class CC_EXPORT LayerTreeHostImpl : public TileManagerClient,
// mutable because |contains_srgb_cache_| is accessed in a const method.
mutable base::MRUCache<gfx::ColorSpace, bool> contains_srgb_cache_;

// When enabled, calculates which frame sinks can be throttled based on
// some pre-defined criteria.
ThrottleDecider throttle_decider_;

// Must be the last member to ensure this is destroyed first in the
// destruction order and invalidates all weak pointers.
base::WeakPtrFactory<LayerTreeHostImpl> weak_factory_{this};
Expand Down
2 changes: 2 additions & 0 deletions cc/trees/layer_tree_host_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,8 @@ class LayerTreeHostImplTest : public testing::Test,
first_scroll_observed++;
}
bool IsInSynchronousComposite() const override { return false; }
void FrameSinksToThrottleUpdated(
const base::flat_set<viz::FrameSinkId>& ids) override {}
void set_reduce_memory_result(bool reduce_memory_result) {
reduce_memory_result_ = reduce_memory_result;
}
Expand Down
8 changes: 8 additions & 0 deletions cc/trees/layer_tree_host_single_thread_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
#ifndef CC_TREES_LAYER_TREE_HOST_SINGLE_THREAD_CLIENT_H_
#define CC_TREES_LAYER_TREE_HOST_SINGLE_THREAD_CLIENT_H_

#include "base/containers/flat_set.h"
#include "base/time/time.h"
#include "components/viz/common/surfaces/frame_sink_id.h"

namespace cc {

Expand All @@ -31,6 +33,12 @@ class LayerTreeHostSingleThreadClient {
// run the machinery to acquire a new LayerTreeFrameSink.
virtual void DidLoseLayerTreeFrameSink() = 0;

// When compositing-based throttling is enabled, this function is called every
// time when a frame composition change has updated the frame sinks to
// throttle.
virtual void FrameSinksToThrottleUpdated(
const base::flat_set<viz::FrameSinkId>& ids) {}

protected:
virtual ~LayerTreeHostSingleThreadClient() {}
};
Expand Down
5 changes: 5 additions & 0 deletions cc/trees/proxy_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,11 @@ bool ProxyImpl::IsInSynchronousComposite() const {
return false;
}

void ProxyImpl::FrameSinksToThrottleUpdated(
const base::flat_set<viz::FrameSinkId>& ids) {
NOTREACHED();
}

void ProxyImpl::NotifyReadyToCommitOnImpl(
CompletionEvent* completion,
LayerTreeHost* layer_tree_host,
Expand Down
4 changes: 3 additions & 1 deletion cc/trees/proxy_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,9 @@ class CC_EXPORT ProxyImpl : public LayerTreeHostImplClient,
void DidObserveFirstScrollDelay(
base::TimeDelta first_scroll_delay,
base::TimeTicks first_scroll_timestamp) override;
bool IsInSynchronousComposite() const override;
void FrameSinksToThrottleUpdated(
const base::flat_set<viz::FrameSinkId>& id) override;

// SchedulerClient implementation
bool WillBeginImplFrame(const viz::BeginFrameArgs& args) override;
Expand All @@ -151,7 +154,6 @@ class CC_EXPORT ProxyImpl : public LayerTreeHostImplClient,
base::TimeTicks time) override;
void FrameIntervalUpdated(base::TimeDelta interval) override {}
bool HasCustomPropertyAnimations() const override;
bool IsInSynchronousComposite() const override;

DrawResult DrawInternal(bool forced_draw);

Expand Down
14 changes: 10 additions & 4 deletions cc/trees/single_thread_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -567,17 +567,23 @@ void SingleThreadProxy::NotifyThroughputTrackerResults(
weak_factory_.GetWeakPtr(), std::move(results)));
}

bool SingleThreadProxy::IsInSynchronousComposite() const {
return inside_synchronous_composite_;
}

void SingleThreadProxy::FrameSinksToThrottleUpdated(
const base::flat_set<viz::FrameSinkId>& ids) {
DebugScopedSetMainThread main(task_runner_provider_);
single_thread_client_->FrameSinksToThrottleUpdated(ids);
}

void SingleThreadProxy::RequestBeginMainFrameNotExpected(bool new_state) {
if (scheduler_on_impl_thread_) {
scheduler_on_impl_thread_->SetMainThreadWantsBeginMainFrameNotExpected(
new_state);
}
}

bool SingleThreadProxy::IsInSynchronousComposite() const {
return inside_synchronous_composite_;
}

void SingleThreadProxy::CompositeImmediatelyForTest(
base::TimeTicks frame_begin_time,
bool raster) {
Expand Down
3 changes: 3 additions & 0 deletions cc/trees/single_thread_proxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <vector>

#include "base/cancelable_callback.h"
#include "base/containers/flat_set.h"
#include "base/time/time.h"
#include "cc/scheduler/scheduler.h"
#include "cc/trees/layer_tree_host_impl.h"
Expand Down Expand Up @@ -140,6 +141,8 @@ class CC_EXPORT SingleThreadProxy : public Proxy,
Scheduler::PaintWorkletState state) override;
void NotifyThroughputTrackerResults(CustomTrackerResults results) override;
bool IsInSynchronousComposite() const override;
void FrameSinksToThrottleUpdated(
const base::flat_set<viz::FrameSinkId>& ids) override;

void RequestNewLayerTreeFrameSink();

Expand Down
Loading

0 comments on commit a98c7e4

Please sign in to comment.