[TimeStats] reduce aggregated stats size for shared timeline-based stats
Shared timeline tracks transaction-level jank, where a transaction is
not necessarily associated with a buffer. But, in general transactions
have debugging names that are not intended to be aggregated if there was
no buffer. E.g., they include things like hash codes, and systemui
generates a lot of different layer names (e.g., there is a separate
animation leash used per app on app launch).
As a workaround, to avoid needing to impose a restriction on all
transaction callers on layer names, we'll do the following:
* Layers that do not have buffers associated with them are collected
into a generic bucket keyed by the app uid, with a layer name == "none".
Because we enforce a minimum layer name, this is suitable as a default
choice for a layer name. This way we can continue to collect
transaction-level jank associated with things like animation leashes.
* Layers that already associated with buffers are collected as normal,
so as to preserve existing data fidelity.
Bug: 173018067
Bug: 171309796
Test: adb shell dumpsys SurfaceFlinger --timestats -enable
Test: adb shell dumpsys SurfaceFlinger --timestats -dump
Change-Id: I8fa57ac99de14f17464d6b5d6bb4d719bd7ad9a3
diff --git a/services/surfaceflinger/TimeStats/TimeStats.cpp b/services/surfaceflinger/TimeStats/TimeStats.cpp
index fe9e737..28a3a81 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.cpp
+++ b/services/surfaceflinger/TimeStats/TimeStats.cpp
@@ -716,23 +716,27 @@
ATRACE_CALL();
std::lock_guard<std::mutex> lock(mMutex);
- // Only update layer stats if we're allowed to do so.
+ // Only update layer stats if we're already tracking the layer in TimeStats.
+ // Otherwise, continue tracking the statistic but use a default layer name instead.
// As an implementation detail, we do this because this method is expected to be
- // called from FrameTimeline, which is allowed to do jank analysis well after a frame is
- // presented. This means that we can't rely on TimeStats to flush layer records over to the
- // aggregated stats.
- if (!canAddNewAggregatedStats(uid, layerName)) {
- return;
- }
+ // called from FrameTimeline, whose jank classification includes transaction jank
+ // that occurs without a buffer. But, in general those layer names are not suitable as
+ // aggregation keys: e.g., it's normal and expected for Window Manager to include the hash code
+ // for an animation leash. So while we can show that jank in dumpsys, aggregating based on the
+ // layer blows up the stats size, so as a workaround drop those stats. This assumes that
+ // TimeStats will flush the first present fence for a layer *before* FrameTimeline does so that
+ // the first jank record is not dropped.
- // Defensively initialize the stats in case FrameTimeline flushes its signaled present fences
- // before TimeStats does.
+ bool useDefaultLayerKey = false;
+ static const std::string kDefaultLayerName = "none";
if (!mTimeStats.stats.count({uid, layerName})) {
- mTimeStats.stats[{uid, layerName}].uid = uid;
- mTimeStats.stats[{uid, layerName}].layerName = layerName;
+ mTimeStats.stats[{uid, kDefaultLayerName}].uid = uid;
+ mTimeStats.stats[{uid, kDefaultLayerName}].layerName = kDefaultLayerName;
+ useDefaultLayerKey = true;
}
- TimeStatsHelper::TimeStatsLayer& timeStatsLayer = mTimeStats.stats[{uid, layerName}];
+ TimeStatsHelper::TimeStatsLayer& timeStatsLayer =
+ mTimeStats.stats[{uid, useDefaultLayerKey ? kDefaultLayerName : layerName}];
updateJankPayload<TimeStatsHelper::TimeStatsLayer>(timeStatsLayer, reasons);
}