Skip to content

Commit

Permalink
Add cc::CommitState to snapshot state for compositor commit
Browse files Browse the repository at this point in the history
Currently, compositor commit accesses LayerTreeHost directly from the
impl thread. This is safe because the main thread is blocked during
commit. However, Non-Blocking Commit is a project to eliminate main
thread blocking during commit; so accessing LayerTreeHost from the
impl thread during commit will no longer be safe.

This CL adds struct CommitState, which contains a snapshot of (almost)
all the information from LayerTreeHost needed for compositor commit. A
bunch of member fields are moved out of LayerTreeHost and into
CommitState; and LayerTreeHost gets pending_commit_state_ and
active_commit_state_. pending_commit_state_ is read and written from
the main thread while an animation frame is being prepared.
active_commit_state_ is passed to the impl thread, which uses it to
drive commit; it is only non-null while commit is running.

Broadly speaking, LayerTreeHost data that is used by commit falls into
two categories: attributes that persist from one commit to the next;
and queues that are drained and passed to commit for processing.
CommitState makes this distinction explicit. Rather than copying data
from the pending state to the active state, LTH::ActivateCommitState()
simply swaps pointers; assigns pending_commit_state_ to a new empty
CommitState; and then copies back all persistent fields from the
now-active CommitState.

With this CL, there are still three data sources used during commit
that are not thread safe: the layer tree, the property trees, and
animation state. The strategy for Non-BlockingCommit is to enforce
thread safety for these data by blocking the main thread only at the
point where the main thread needs to modify them. Subsequent CL's will
do that by adding calls to LTH::WaitForCommitCompletion() where
necessary.

Bug: 1237973,1255972
Change-Id: I3e9742e87539632ba1b0a7a13a79ccc5e08df1c7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3228195
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Commit-Queue: Stefan Zager <szager@chromium.org>
Cr-Commit-Position: refs/heads/main@{#936106}
  • Loading branch information
szager-chromium authored and Chromium LUCI CQ committed Oct 28, 2021
1 parent 33e336a commit 40f911d
Show file tree
Hide file tree
Showing 81 changed files with 1,729 additions and 1,145 deletions.
3 changes: 3 additions & 0 deletions cc/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,8 @@ cc_component("cc") {
"trees/clip_expander.h",
"trees/clip_node.cc",
"trees/clip_node.h",
"trees/commit_state.cc",
"trees/commit_state.h",
"trees/compositor_commit_data.cc",
"trees/compositor_commit_data.h",
"trees/compositor_mode.h",
Expand Down Expand Up @@ -434,6 +436,7 @@ cc_component("cc") {
"trees/ukm_manager.h",
"trees/viewport_layers.cc",
"trees/viewport_layers.h",
"trees/viewport_property_ids.h",
]

public_deps = [
Expand Down
8 changes: 5 additions & 3 deletions cc/benchmarks/micro_benchmark_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,9 @@ bool MicroBenchmarkController::SendMessage(int id, base::Value message) {
return (*it)->ProcessMessage(std::move(message));
}

void MicroBenchmarkController::ScheduleImplBenchmarks(
LayerTreeHostImpl* host_impl) {
std::vector<std::unique_ptr<MicroBenchmarkImpl>>
MicroBenchmarkController::CreateImplBenchmarks() const {
std::vector<std::unique_ptr<MicroBenchmarkImpl>> result;
for (const auto& benchmark : benchmarks_) {
std::unique_ptr<MicroBenchmarkImpl> benchmark_impl;
if (!benchmark->ProcessedForBenchmarkImpl()) {
Expand All @@ -97,8 +98,9 @@ void MicroBenchmarkController::ScheduleImplBenchmarks(
}

if (benchmark_impl.get())
host_impl->ScheduleMicroBenchmark(std::move(benchmark_impl));
result.push_back(std::move(benchmark_impl));
}
return result;
}

void MicroBenchmarkController::DidUpdateLayers() {
Expand Down
4 changes: 2 additions & 2 deletions cc/benchmarks/micro_benchmark_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class Value;
namespace cc {

class LayerTreeHost;
class LayerTreeHostImpl;

class CC_EXPORT MicroBenchmarkController {
public:
explicit MicroBenchmarkController(LayerTreeHost* host);
Expand All @@ -38,7 +38,7 @@ class CC_EXPORT MicroBenchmarkController {
// Returns true if the message was successfully delivered and handled.
bool SendMessage(int id, base::Value message);

void ScheduleImplBenchmarks(LayerTreeHostImpl* host_impl);
std::vector<std::unique_ptr<MicroBenchmarkImpl>> CreateImplBenchmarks() const;

private:
void CleanUpFinishedBenchmarks();
Expand Down
11 changes: 6 additions & 5 deletions cc/benchmarks/micro_benchmark_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,12 @@ TEST_F(MicroBenchmarkControllerTest, BenchmarkImplRan) {
base::BindOnce(&IncrementCallCount, base::Unretained(&run_count)));
EXPECT_GT(id, 0);

// Schedule impl benchmarks. In production code, this is run in commit.
layer_tree_host_->GetMicroBenchmarkController()->ScheduleImplBenchmarks(
layer_tree_host_impl_.get());

// Now complete the commit (as if on the impl thread).
// Scheduling benchmarks on the impl thread is usually done during
// LayerTreeHostImpl::FinishCommit().
for (auto& benchmark : layer_tree_host_->GetMicroBenchmarkController()
->CreateImplBenchmarks()) {
layer_tree_host_impl_->ScheduleMicroBenchmark(std::move(benchmark));
}
layer_tree_host_impl_->CommitComplete();

// Make sure all posted messages run.
Expand Down
5 changes: 3 additions & 2 deletions cc/layers/heads_up_display_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,9 @@ void HeadsUpDisplayLayer::UpdateWebVitalMetrics(
web_vital_metrics_ = std::move(web_vital_metrics);
}

void HeadsUpDisplayLayer::PushPropertiesTo(LayerImpl* layer) {
Layer::PushPropertiesTo(layer);
void HeadsUpDisplayLayer::PushPropertiesTo(LayerImpl* layer,
const CommitState& commit_state) {
Layer::PushPropertiesTo(layer, commit_state);
TRACE_EVENT0("cc", "HeadsUpDisplayLayer::PushPropertiesTo");
HeadsUpDisplayLayerImpl* layer_impl =
static_cast<HeadsUpDisplayLayerImpl*>(layer);
Expand Down
3 changes: 2 additions & 1 deletion cc/layers/heads_up_display_layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ class CC_EXPORT HeadsUpDisplayLayer : public Layer {
std::unique_ptr<LayerImpl> CreateLayerImpl(LayerTreeImpl* tree_impl) override;

// Layer overrides.
void PushPropertiesTo(LayerImpl* layer) override;
void PushPropertiesTo(LayerImpl* layer,
const CommitState& commit_state) override;

protected:
HeadsUpDisplayLayer();
Expand Down
20 changes: 16 additions & 4 deletions cc/layers/layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ sk_sp<SkPicture> Layer::GetPicture() const {
}

void Layer::SetParent(Layer* layer) {
DCHECK(IsMutationAllowed());
DCHECK(!layer || !layer->HasAncestor(this));

parent_ = layer;
Expand Down Expand Up @@ -397,6 +398,7 @@ void Layer::RemoveAllChildren() {

void Layer::SetChildLayerList(LayerList new_children) {
DCHECK(layer_tree_host_->IsUsingLayerLists());
DCHECK(IsMutationAllowed());

// Early out without calling |LayerTreeHost::SetNeedsFullTreeSync| if no
// layer has changed.
Expand Down Expand Up @@ -507,7 +509,7 @@ void Layer::SetSafeOpaqueBackgroundColor(SkColor background_color) {
SetNeedsPushProperties();
}

SkColor Layer::SafeOpaqueBackgroundColor() const {
SkColor Layer::SafeOpaqueBackgroundColor(SkColor host_background_color) const {
if (contents_opaque()) {
if (!layer_tree_host_ || !layer_tree_host_->IsUsingLayerLists()) {
// In layer tree mode, PropertyTreeBuilder should have calculated the safe
Expand All @@ -522,7 +524,7 @@ SkColor Layer::SafeOpaqueBackgroundColor() const {
// background_color() if it's not transparent, or layer_tree_host_'s
// background_color(), with the alpha channel forced to be opaque.
SkColor color = background_color() == SK_ColorTRANSPARENT
? layer_tree_host_->background_color()
? host_background_color
: background_color();
return SkColorSetA(color, SK_AlphaOPAQUE);
}
Expand All @@ -536,6 +538,14 @@ SkColor Layer::SafeOpaqueBackgroundColor() const {
return background_color();
}

SkColor Layer::SafeOpaqueBackgroundColor() const {
SkColor host_background_color =
layer_tree_host_
? layer_tree_host_->pending_commit_state()->background_color
: layer_tree_inputs()->safe_opaque_background_color;
return SafeOpaqueBackgroundColor(host_background_color);
}

void Layer::SetMasksToBounds(bool masks_to_bounds) {
DCHECK(IsPropertyChangeAllowed());
auto& inputs = EnsureLayerTreeInputs();
Expand Down Expand Up @@ -1339,7 +1349,8 @@ bool Layer::IsSnappedToPixelGridInTarget() {
return false;
}

void Layer::PushPropertiesTo(LayerImpl* layer) {
void Layer::PushPropertiesTo(LayerImpl* layer,
const CommitState& commit_state) {
TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
"Layer::PushPropertiesTo");
DCHECK(layer_tree_host_);
Expand All @@ -1350,7 +1361,8 @@ void Layer::PushPropertiesTo(LayerImpl* layer) {
layer->SetElementId(inputs_.element_id);
layer->SetHasTransformNode(has_transform_node_);
layer->SetBackgroundColor(inputs_.background_color);
layer->SetSafeOpaqueBackgroundColor(SafeOpaqueBackgroundColor());
layer->SetSafeOpaqueBackgroundColor(
SafeOpaqueBackgroundColor(commit_state.background_color));
layer->SetBounds(inputs_.bounds);
layer->SetTransformTreeIndex(transform_tree_index());
layer->SetEffectTreeIndex(effect_tree_index());
Expand Down
11 changes: 9 additions & 2 deletions cc/layers/layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ class LayerTreeHostCommon;
class LayerTreeImpl;
class PictureLayer;

struct CommitState;

// For tracing and debugging. The info will be attached to this layer's tracing
// output.
struct CC_EXPORT LayerDebugInfo {
Expand Down Expand Up @@ -161,11 +163,15 @@ class CC_EXPORT Layer : public base::RefCounted<Layer> {
// If the layer says contents_opaque() is true, in layer tree mode, this
// returns the value set by SetSafeOpaqueBackgroundColor() which should be an
// opaque color, and in layer list mode, returns an opaque color calculated
// from background_color() and layer_tree_host()->background_color().
// from background_color() and the argument host_background_color.
// Otherwise, it returns something non-opaque. It prefers to return the
// background_color(), but if the background_color() is opaque (and this layer
// claims to not be), then SK_ColorTRANSPARENT is returned to avoid intrusive
// checkerboard where the layer is not covered by the background_color().
SkColor SafeOpaqueBackgroundColor(SkColor host_background_color) const;

// Same as the one-argument version, except that host_background_color is
// layer_tree_host()->pending_commit_state()->background_color.
SkColor SafeOpaqueBackgroundColor() const;

// For layer tree mode only.
Expand Down Expand Up @@ -632,7 +638,8 @@ class CC_EXPORT Layer : public base::RefCounted<Layer> {
// that state as well. The |layer| passed in will be of the type created by
// CreateLayerImpl(), so can be safely down-casted if the subclass uses a
// different type for the compositor thread.
virtual void PushPropertiesTo(LayerImpl* layer);
virtual void PushPropertiesTo(LayerImpl* layer,
const CommitState& commit_state);

// Internal method to be overridden by Layer subclasses that need to do work
// during a main frame. The method should compute any state that will need to
Expand Down
10 changes: 8 additions & 2 deletions cc/layers/layer_perftest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,12 @@ TEST_F(LayerPerfTest, PushPropertiesTo) {
test_layer->SetContentsOpaque(contents_opaque);
test_layer->SetHideLayerAndSubtree(hide_layer_and_subtree);
test_layer->SetMasksToBounds(masks_to_bounds);
test_layer->PushPropertiesTo(impl_layer.get());
// Here and elsewhere: when doing a full commit, we would call
// layer_tree_host_->ActivateCommitState() and the second argument would
// come from layer_tree_host_->active_commit_state(); we use
// pending_commit_state() just to keep the test code simple.
test_layer->PushPropertiesTo(impl_layer.get(),
*layer_tree_host_->pending_commit_state());

transform_origin_z += 0.01f;
scrollable = !scrollable;
Expand All @@ -103,7 +108,8 @@ TEST_F(LayerPerfTest, PushPropertiesTo) {
// Properties didn't change.
timer_.Reset();
do {
test_layer->PushPropertiesTo(impl_layer.get());
test_layer->PushPropertiesTo(impl_layer.get(),
*layer_tree_host_->pending_commit_state());
timer_.NextLap();
} while (!timer_.HasTimeLimitExpired());

Expand Down
Loading

0 comments on commit 40f911d

Please sign in to comment.