Clean up hung_commit debug info.
This debug code (crash key, trace event, and UMA histogram) was added
for crbug.com/1378021 which has since been resolved.
Bug: 1369739
Change-Id: I93662abe6d63e21d5dee0aa13408cedc268b0b96
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4602248
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Reviewed-by: Vladimir Levin <vmpstr@chromium.org>
Commit-Queue: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1156454}
diff --git a/cc/scheduler/scheduler.cc b/cc/scheduler/scheduler.cc
index 60fa3447..02883cab 100644
--- a/cc/scheduler/scheduler.cc
+++ b/cc/scheduler/scheduler.cc
@@ -211,7 +211,6 @@
state_machine_.NotifyReadyToCommit();
next_commit_origin_frame_args_ = last_dispatched_begin_main_frame_args_;
}
- trace_actions_ = true;
ProcessScheduledActions();
}
@@ -869,10 +868,6 @@
SchedulerStateMachine::Action action;
do {
action = state_machine_.NextAction();
-
- if (trace_actions_ && action != SchedulerStateMachine::Action::NONE &&
- commit_debug_action_sequence_.size() < 40)
- commit_debug_action_sequence_.push_back(action);
TRACE_EVENT(TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler"),
"SchedulerStateMachine", [this](perfetto::EventContext ctx) {
this->AsProtozeroInto(ctx,
@@ -906,8 +901,6 @@
state_machine_.WillCommit(/*commit_had_no_updates=*/false);
compositor_timing_history_->WillCommit();
compositor_frame_reporting_controller_->WillCommit();
- commit_debug_action_sequence_.clear();
- trace_actions_ = false;
client_->ScheduledActionCommit();
compositor_timing_history_->DidCommit();
compositor_frame_reporting_controller_->DidCommit();
@@ -1058,47 +1051,4 @@
}
}
-std::string Scheduler::GetHungCommitDebugInfo() const {
- // Convert the stored actions into a debug string.
- std::string sequence;
- // We convert each action to a char 'a' plus the enum value. So we only need
- // the number of actions we're outputting.
- sequence.reserve(commit_debug_action_sequence_.size());
- for (auto action : commit_debug_action_sequence_) {
- sequence.push_back('a' + static_cast<int>(action));
- }
- return base::StringPrintf(
- "a[a%s] bmfs%d hpt%d atnfd%d pw%d aw%d rfa%d", sequence.c_str(),
- static_cast<int>(state_machine_.begin_main_frame_state()),
- static_cast<int>(state_machine_.has_pending_tree()),
- static_cast<int>(state_machine_.active_tree_needs_first_draw()),
- static_cast<int>(
- state_machine_.processing_paint_worklets_for_pending_tree()),
- static_cast<int>(
- state_machine_.processing_animation_worklets_for_pending_tree()),
- static_cast<int>(state_machine_.pending_tree_is_ready_for_activation()));
-}
-
-void Scheduler::TraceHungCommitDebugInfo() const {
- // First output a series of events which have the old actions.
- for (auto action : commit_debug_action_sequence_) {
- TRACE_EVENT_INSTANT(
- "cc", "ProxyImpl::OnHungCommit OldAction",
- [action](perfetto::EventContext ctx) {
- ctx.event()
- ->set_cc_scheduler_state()
- ->set_state_machine()
- ->set_major_state()
- ->set_next_action(
- SchedulerStateMachine::ActionToProtozeroEnum(action));
- });
- }
- // Finally dump the complete state of the scheduler.
- TRACE_EVENT_INSTANT("cc", "ProxyImpl::OnHungCommit CurrentState",
- [this](perfetto::EventContext ctx) {
- this->AsProtozeroInto(
- ctx, ctx.event()->set_cc_scheduler_state());
- });
-}
-
} // namespace cc
diff --git a/cc/scheduler/scheduler.h b/cc/scheduler/scheduler.h
index 902cf389..eb645bf38 100644
--- a/cc/scheduler/scheduler.h
+++ b/cc/scheduler/scheduler.h
@@ -280,9 +280,6 @@
size_t CommitDurationSampleCountForTesting() const;
- std::string GetHungCommitDebugInfo() const;
- void TraceHungCommitDebugInfo() const;
-
protected:
// Virtual for testing.
virtual base::TimeTicks Now() const;
@@ -407,10 +404,6 @@
}
void UpdatePowerModeVote();
-
- // Temporary for production debugging of renderer hang (crbug.com/1159366).
- std::vector<SchedulerStateMachine::Action> commit_debug_action_sequence_;
- bool trace_actions_ = false;
};
} // namespace cc
diff --git a/cc/tiles/tile_manager.cc b/cc/tiles/tile_manager.cc
index c5a51c1..3ebce5d6 100644
--- a/cc/tiles/tile_manager.cc
+++ b/cc/tiles/tile_manager.cc
@@ -1881,12 +1881,6 @@
global_state_.memory_limit_policy != ALLOW_ABSOLUTE_MINIMUM);
}
-std::string TileManager::GetHungCommitDebugInfo() const {
- base::trace_event::TracedValueJSON value;
- ActivationStateAsValueInto(&value);
- return value.ToJSON();
-}
-
TileManager::MemoryUsage::MemoryUsage()
: memory_bytes_(0), resource_count_(0) {}
diff --git a/cc/tiles/tile_manager.h b/cc/tiles/tile_manager.h
index 89b8ddb5..55a0d27 100644
--- a/cc/tiles/tile_manager.h
+++ b/cc/tiles/tile_manager.h
@@ -10,7 +10,6 @@
#include <memory>
#include <set>
-#include <string>
#include <unordered_map>
#include <unordered_set>
#include <utility>
@@ -325,8 +324,6 @@
void set_active_url(const GURL& url) { active_url_ = url; }
- std::string GetHungCommitDebugInfo() const;
-
protected:
friend class Tile;
// Must be called by tile during destruction.
diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
index d40b7e9..943e321 100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -5277,21 +5277,4 @@
frame_token, std::move(callbacks));
}
-std::string LayerTreeHostImpl::GetHungCommitDebugInfo() const {
- return base::StringPrintf(
- "ptfp%d pwpd%d tmrta%d as%d gpur%d%d ltfs%d%d zc%d ",
- static_cast<int>(pending_tree_fully_painted_),
- static_cast<int>(paint_worklet_painter_ &&
- paint_worklet_painter_->HasOngoingDispatch()),
- static_cast<int>(tile_manager_.IsReadyToActivate()),
- static_cast<int>(GetActivelyScrollingType()),
- static_cast<int>(raster_caps().use_gpu_rasterization),
- static_cast<int>(raster_caps().gpu_rasterization_status),
- static_cast<int>(has_valid_layer_tree_frame_sink_),
- static_cast<int>(layer_tree_frame_sink_ &&
- layer_tree_frame_sink_->context_provider()),
- static_cast<int>(settings_.use_zero_copy)) +
- tile_manager_.GetHungCommitDebugInfo();
-}
-
} // namespace cc
diff --git a/cc/trees/layer_tree_host_impl.h b/cc/trees/layer_tree_host_impl.h
index 15d27cc..27866cc 100644
--- a/cc/trees/layer_tree_host_impl.h
+++ b/cc/trees/layer_tree_host_impl.h
@@ -914,8 +914,6 @@
downsample_metrics_ = value;
}
- std::string GetHungCommitDebugInfo() const;
-
protected:
LayerTreeHostImpl(
const LayerTreeSettings& settings,
diff --git a/cc/trees/proxy_impl.cc b/cc/trees/proxy_impl.cc
index 27124ea..59e3aa3 100644
--- a/cc/trees/proxy_impl.cc
+++ b/cc/trees/proxy_impl.cc
@@ -13,7 +13,6 @@
#include <vector>
#include "base/auto_reset.h"
-#include "base/debug/crash_logging.h"
#include "base/functional/bind.h"
#include "base/memory/raw_ptr.h"
#include "base/notreached.h"
@@ -49,9 +48,6 @@
constexpr auto kSmoothnessTakesPriorityExpirationDelay =
base::Milliseconds(250);
-// Make this less than kHungRendererDelay (15 sec).
-constexpr base::TimeDelta kHungCommitTimeout = base::Seconds(14);
-
} // namespace
// Ensures that a CompletionEvent for commit is always signaled.
@@ -379,9 +375,6 @@
completion_event, start_time, MainThreadTaskRunner(),
proxy_main_weak_ptr_),
std::move(commit_state), unsafe_state, commit_timestamps);
- hung_commit_timer_.Start(
- FROM_HERE, kHungCommitTimeout,
- base::BindOnce(&ProxyImpl::OnHungCommit, base::Unretained(this)));
// Extract metrics data from the layer tree host and send them to the
// scheduler to pass them to the compositor_timing_history object.
@@ -393,17 +386,6 @@
scheduler_->SetNeedsBeginMainFrame();
}
-void ProxyImpl::OnHungCommit() {
- UMA_HISTOGRAM_BOOLEAN("Compositing.Renderer.CommitHung", true);
- static auto* hung_commit_data = base::debug::AllocateCrashKeyString(
- "hung_commit", base::debug::CrashKeySize::Size1024);
- std::string debug_info = host_impl_->GetHungCommitDebugInfo() +
- scheduler_->GetHungCommitDebugInfo();
- LOG(ERROR) << "commit hung: " << debug_info;
- base::debug::SetCrashKeyString(hung_commit_data, debug_info);
- scheduler_->TraceHungCommitDebugInfo();
-}
-
void ProxyImpl::DidLoseLayerTreeFrameSinkOnImplThread() {
TRACE_EVENT0("cc", "ProxyImpl::DidLoseLayerTreeFrameSinkOnImplThread");
DCHECK(IsImplThread());
@@ -817,7 +799,6 @@
}
data_for_commit_.reset();
- hung_commit_timer_.Stop();
}
void ProxyImpl::ScheduledActionPostCommit() {
diff --git a/cc/trees/proxy_impl.h b/cc/trees/proxy_impl.h
index 1bd8393..c27b306 100644
--- a/cc/trees/proxy_impl.h
+++ b/cc/trees/proxy_impl.h
@@ -166,8 +166,6 @@
base::SingleThreadTaskRunner* MainThreadTaskRunner();
bool ShouldDeferBeginMainFrame() const;
- void OnHungCommit();
-
const int layer_tree_host_id_;
std::unique_ptr<Scheduler> scheduler_;
@@ -223,9 +221,6 @@
// Either thread can request deferring BeginMainFrame; keep track of both.
bool main_wants_defer_begin_main_frame_ = false;
bool impl_wants_defer_begin_main_frame_ = false;
-
- // Temporary for production debugging of renderer hang (crbug.com/1159366).
- base::OneShotTimer hung_commit_timer_;
};
} // namespace cc
diff --git a/tools/metrics/histograms/metadata/compositing/histograms.xml b/tools/metrics/histograms/metadata/compositing/histograms.xml
index 3374300b..02cf643 100644
--- a/tools/metrics/histograms/metadata/compositing/histograms.xml
+++ b/tools/metrics/histograms/metadata/compositing/histograms.xml
@@ -686,16 +686,6 @@
</summary>
</histogram>
-<histogram name="Compositing.Renderer.CommitHung" units="boolean"
- expires_after="2023-11-19">
- <owner>skobes@chromium.org</owner>
- <owner>input-dev@chromium.org</owner>
- <summary>
- Records when the renderer compositor thread is hung (14 seconds) waiting to
- run the commit after the main thread signalled that it is ready to commit.
- </summary>
-</histogram>
-
<histogram name="Compositing.Renderer.LayersUpdateTime" units="microseconds"
expires_after="2023-11-12">
<owner>pdr@chromium.org</owner>
