From 4824afe5386ce9130f3ac57981310f484b387a8a Mon Sep 17 00:00:00 2001 From: Vadim Tryshev Date: Wed, 26 Jul 2023 11:59:15 -0700 Subject: [PATCH] Optimizing application of the ignore-list of nodes for AlphaJumpDetector. Instead of building the full path of each node, and then searching it in the PATHS_TO_IGNORE set, we are descending the tree of the nodes to ignore. This saves us building the whole path for each node. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that AlphaJumpDetector has more fields specifically for alpha jump detection in AnalysisNode, I’ve introduced an abstraction of per-detector data in AnalysisNode, see the ‘detectorsData’ field. Each detector (alpha jumps, flash, position jump), will be able to add its own data to AnalysisNode without polluting it. Flag: N/A Test: presubmit, local runs Bug: 286251603 Change-Id: Iac8504edfe43407a75e7fc4a39e21bfca502b090 --- .../AlphaJumpDetector.java | 82 +++++++++++++++++-- .../ViewCaptureAnalyzer.java | 38 ++++++--- 2 files changed, 99 insertions(+), 21 deletions(-) diff --git a/tests/src/com/android/launcher3/util/viewcapture_analysis/AlphaJumpDetector.java b/tests/src/com/android/launcher3/util/viewcapture_analysis/AlphaJumpDetector.java index 2501801c42..fdc70e2133 100644 --- a/tests/src/com/android/launcher3/util/viewcapture_analysis/AlphaJumpDetector.java +++ b/tests/src/com/android/launcher3/util/viewcapture_analysis/AlphaJumpDetector.java @@ -15,13 +15,12 @@ */ package com.android.launcher3.util.viewcapture_analysis; -import static com.android.launcher3.util.viewcapture_analysis.ViewCaptureAnalyzer.diagPathFromRoot; - import com.android.launcher3.util.viewcapture_analysis.ViewCaptureAnalyzer.AnalysisNode; import com.android.launcher3.util.viewcapture_analysis.ViewCaptureAnalyzer.AnomalyDetector; -import java.util.Collection; -import java.util.Set; +import java.util.HashMap; +import java.util.List; +import java.util.Map; /** * Anomaly detector that triggers an error when alpha of a view changes too rapidly. @@ -34,7 +33,7 @@ final class AlphaJumpDetector extends AnomalyDetector { CONTENT + "LauncherRootView:id/launcher|DragLayer:id/drag_layer|"; // Paths of nodes that are excluded from analysis. - private static final Collection PATHS_TO_IGNORE = Set.of( + private static final Iterable PATHS_TO_IGNORE = List.of( CONTENT + "AddItemDragLayer:id/add_item_drag_layer|AddItemWidgetsBottomSheet:id" + "/add_item_bottom_sheet|LinearLayout:id/add_item_bottom_sheet_content" @@ -124,15 +123,80 @@ final class AlphaJumpDetector extends AnomalyDetector { + ":id/overview_actions_view|LinearLayout:id/action_buttons|Button:id" + "/action_select" ); + + /** + * Element of the tree of ignored nodes. + * If the "children" map is empty, then this node should be ignored, i.e. alpha jumps analysis + * shouldn't run for it. + * I.e. ignored nodes correspond to the leaves in the ignored nodes tree. + */ + private static class IgnoreNode { + // Map from child node identities to ignore-nodes for these children. + public final Map children = new HashMap<>(); + } + + private static final IgnoreNode IGNORED_NODES_ROOT = buildIgnoreNodesTree(); + + // Converts the list of full paths of nodes to ignore to a more efficient tree of ignore-nodes. + private static IgnoreNode buildIgnoreNodesTree() { + final IgnoreNode root = new IgnoreNode(); + for (String pathToIgnore : PATHS_TO_IGNORE) { + // Scan the diag path of an ignored node and add its elements into the tree. + IgnoreNode currentIgnoreNode = root; + for (String part : pathToIgnore.split("\\|")) { + // Ensure that the child of the node is added to the tree. + IgnoreNode child = currentIgnoreNode.children.get(part); + if (child == null) { + currentIgnoreNode.children.put(part, child = new IgnoreNode()); + } + currentIgnoreNode = child; + } + } + return root; + } + // Minimal increase or decrease of view's alpha between frames that triggers the error. private static final float ALPHA_JUMP_THRESHOLD = 1f; + // Per-AnalysisNode data that's specific to this detector. + private static class NodeData { + public boolean ignoreAlphaJumps; + + // If ignoreNode is null, then this AnalysisNode node will be ignored if its parent is + // ignored. + // Otherwise, this AnalysisNode will be ignored if ignoreNode is a leaf i.e. has no + // children. + public IgnoreNode ignoreNode; + } + + private NodeData getNodeData(AnalysisNode info) { + return (NodeData) info.detectorsData[detectorOrdinal]; + } + @Override void initializeNode(AnalysisNode info) { + final NodeData nodeData = new NodeData(); + info.detectorsData[detectorOrdinal] = nodeData; + // If the parent view ignores alpha jumps, its descendants will too. - final boolean parentIgnoreAlphaJumps = info.parent != null && info.parent.ignoreAlphaJumps; - info.ignoreAlphaJumps = parentIgnoreAlphaJumps - || PATHS_TO_IGNORE.contains(diagPathFromRoot(info)); + final boolean parentIgnoresAlphaJumps = info.parent != null && getNodeData( + info.parent).ignoreAlphaJumps; + if (parentIgnoresAlphaJumps) { + nodeData.ignoreAlphaJumps = true; + return; + } + + // Parent view doesn't ignore alpha jumps. + // Initialize this AnalysisNode's ignore-node with the corresponding child of the + // ignore-node of the parent, if present. + final IgnoreNode parentIgnoreNode = info.parent != null + ? getNodeData(info.parent).ignoreNode + : IGNORED_NODES_ROOT; + nodeData.ignoreNode = parentIgnoreNode != null + ? parentIgnoreNode.children.get(info.nodeIdentity) : null; + // AnalysisNode will be ignored if the corresponding ignore-node is a leaf. + nodeData.ignoreAlphaJumps = + nodeData.ignoreNode != null && nodeData.ignoreNode.children.isEmpty(); } @Override @@ -142,7 +206,7 @@ final class AlphaJumpDetector extends AnomalyDetector { if (oldInfo != null && oldInfo.frameN != frameN) return; final AnalysisNode latestInfo = newInfo != null ? newInfo : oldInfo; - if (latestInfo.ignoreAlphaJumps) return; + if (getNodeData(latestInfo).ignoreAlphaJumps) return; final float oldAlpha = oldInfo != null ? oldInfo.alpha : 0; final float newAlpha = newInfo != null ? newInfo.alpha : 0; diff --git a/tests/src/com/android/launcher3/util/viewcapture_analysis/ViewCaptureAnalyzer.java b/tests/src/com/android/launcher3/util/viewcapture_analysis/ViewCaptureAnalyzer.java index 5a2611c362..2cf38437f5 100644 --- a/tests/src/com/android/launcher3/util/viewcapture_analysis/ViewCaptureAnalyzer.java +++ b/tests/src/com/android/launcher3/util/viewcapture_analysis/ViewCaptureAnalyzer.java @@ -40,6 +40,9 @@ public class ViewCaptureAnalyzer { * Detector of one kind of anomaly. */ abstract static class AnomalyDetector { + // Index of this detector in ViewCaptureAnalyzer.ANOMALY_DETECTORS + public int detectorOrdinal; + /** * Initializes fields of the node that are specific to the anomaly detected by this * detector. @@ -64,9 +67,13 @@ public class ViewCaptureAnalyzer { } // All detectors. They will be invoked in the order listed here. - private static final Iterable ANOMALY_DETECTORS = Arrays.asList( + private static final AnomalyDetector[] ANOMALY_DETECTORS = { new AlphaJumpDetector() - ); + }; + + static { + for (int i = 0; i < ANOMALY_DETECTORS.length; ++i) ANOMALY_DETECTORS[i].detectorOrdinal = i; + } // A view from view capture data converted to a form that's convenient for detecting anomalies. static class AnalysisNode { @@ -86,7 +93,11 @@ public class ViewCaptureAnalyzer { public int frameN; public ViewNode viewCaptureNode; - public boolean ignoreAlphaJumps; + // Class name + resource id + public String nodeIdentity; + + // Collection of detector-specific data for this node. + public final Object[] detectorsData = new Object[ANOMALY_DETECTORS.length]; @Override public String toString() { @@ -139,7 +150,7 @@ public class ViewCaptureAnalyzer { for (AnalysisNode info : lastSeenNodes.values()) { if (info.frameN == frameN - 1) { if (!info.viewCaptureNode.getWillNotDraw()) { - ANOMALY_DETECTORS.forEach( + Arrays.stream(ANOMALY_DETECTORS).forEach( detector -> detector.detectAnomalies( /* oldInfo = */ info, /* newInfo = */ null, @@ -177,6 +188,8 @@ public class ViewCaptureAnalyzer { final AnalysisNode newAnalysisNode = new AnalysisNode(); newAnalysisNode.className = viewCaptureData.getClassname(classIndex); newAnalysisNode.resourceId = viewCaptureNode.getId(); + newAnalysisNode.nodeIdentity = + getNodeIdentity(newAnalysisNode.className, newAnalysisNode.resourceId); newAnalysisNode.parent = parent; newAnalysisNode.left = left; newAnalysisNode.top = top; @@ -185,12 +198,13 @@ public class ViewCaptureAnalyzer { newAnalysisNode.alpha = alpha; newAnalysisNode.frameN = frameN; newAnalysisNode.viewCaptureNode = viewCaptureNode; - ANOMALY_DETECTORS.forEach(detector -> detector.initializeNode(newAnalysisNode)); + Arrays.stream(ANOMALY_DETECTORS).forEach( + detector -> detector.initializeNode(newAnalysisNode)); // Detect anomalies for the view final AnalysisNode oldAnalysisNode = lastSeenNodes.get(hashcode); // may be null if (frameN != 0 && !viewCaptureNode.getWillNotDraw()) { - ANOMALY_DETECTORS.forEach( + Arrays.stream(ANOMALY_DETECTORS).forEach( detector -> detector.detectAnomalies(oldAnalysisNode, newAnalysisNode, frameN)); } lastSeenNodes.put(hashcode, newAnalysisNode); @@ -221,18 +235,18 @@ public class ViewCaptureAnalyzer { return className.substring(className.lastIndexOf(".") + 1); } - static String diagPathFromRoot(AnalysisNode nodeBox) { - final StringBuilder path = new StringBuilder(diagPathElement(nodeBox)); + private static String diagPathFromRoot(AnalysisNode nodeBox) { + final StringBuilder path = new StringBuilder(nodeBox.nodeIdentity); for (AnalysisNode ancestor = nodeBox.parent; ancestor != null; ancestor = ancestor.parent) { - path.insert(0, diagPathElement(ancestor) + "|"); + path.insert(0, ancestor.nodeIdentity + "|"); } return path.toString(); } - private static String diagPathElement(AnalysisNode nodeBox) { + private static String getNodeIdentity(String className, String resourceId) { final StringBuilder sb = new StringBuilder(); - sb.append(classNameToSimpleName(nodeBox.className)); - if (!"NO_ID".equals(nodeBox.resourceId)) sb.append(":" + nodeBox.resourceId); + sb.append(classNameToSimpleName(className)); + if (!"NO_ID".equals(resourceId)) sb.append(":" + resourceId); return sb.toString(); } }