From df0b8e588657517b6039b98f8856380372147602 Mon Sep 17 00:00:00 2001 From: Michael Comella Date: Wed, 8 Sep 2021 14:30:49 -0700 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/21183: remove view hierarchy depth check from excessive resource test. This doesn't seem to be a high value test: increasing the view hierarchy depth will only result in a performance problem on low end devices if there is enough content on the new layer to cause the traversal to take longer. It's more likely to result in a hard-to-workaround false positive so we can remove it, like component init count. --- .../perf/StartupExcessiveResourceUseTest.kt | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/app/src/androidTest/java/org/mozilla/fenix/perf/StartupExcessiveResourceUseTest.kt b/app/src/androidTest/java/org/mozilla/fenix/perf/StartupExcessiveResourceUseTest.kt index e79b74652..c39dfd997 100644 --- a/app/src/androidTest/java/org/mozilla/fenix/perf/StartupExcessiveResourceUseTest.kt +++ b/app/src/androidTest/java/org/mozilla/fenix/perf/StartupExcessiveResourceUseTest.kt @@ -4,7 +4,6 @@ package org.mozilla.fenix.perf -import android.util.Log import android.view.View import android.view.ViewGroup import android.widget.LinearLayout @@ -25,7 +24,6 @@ import org.mozilla.fenix.helpers.HomeActivityTestRule private const val EXPECTED_SUPPRESSION_COUNT = 19 @Suppress("TopLevelPropertyNaming") // it's silly this would have a different naming convention b/c no const private val EXPECTED_RUNBLOCKING_RANGE = 0..1 // CI has +1 counts compared to local runs: increment these together -private const val EXPECTED_VIEW_HIERARCHY_DEPTH = 12 private const val EXPECTED_RECYCLER_VIEW_CONSTRAINT_LAYOUT_CHILDREN = 4 private const val EXPECTED_NUMBER_OF_INFLATION = 12 @@ -39,12 +37,6 @@ private val failureMsgRunBlocking = getErrorMessage( implications = "using runBlocking may block the main thread and have other negative performance implications?" ) -private val failureMsgViewHierarchyDepth = getErrorMessage( - shortName = "view hierarchy depth", - implications = "having a deep view hierarchy can slow down measure/layout performance?" -) + "Please note that we're not sure if this is a useful metric to assert: with your feedback, " + - "we'll find out over time if it is or is not." - private val failureMsgRecyclerViewConstraintLayoutChildren = getErrorMessage( shortName = "ConstraintLayout being a common direct descendant of a RecyclerView", implications = "ConstraintLayouts are slow to inflate and are primarily used to flatten deep " + @@ -89,14 +81,12 @@ class StartupExcessiveResourceUseTest { val actualRunBlocking = RunBlockingCounter.count.get() val rootView = activityTestRule.activity.findViewById(R.id.rootContainer) - val actualViewHierarchyDepth = countAndLogViewHierarchyDepth(rootView, 1) val actualRecyclerViewConstraintLayoutChildren = countRecyclerViewConstraintLayoutChildren(rootView, null) val actualNumberOfInflations = InflationCounter.inflationCount.get() assertEquals(failureMsgStrictMode, EXPECTED_SUPPRESSION_COUNT, actualSuppresionCount) assertTrue(failureMsgRunBlocking + "actual: $actualRunBlocking", actualRunBlocking in EXPECTED_RUNBLOCKING_RANGE) - assertEquals(failureMsgViewHierarchyDepth, EXPECTED_VIEW_HIERARCHY_DEPTH, actualViewHierarchyDepth) assertEquals( failureMsgRecyclerViewConstraintLayoutChildren, EXPECTED_RECYCLER_VIEW_CONSTRAINT_LAYOUT_CHILDREN, @@ -106,19 +96,6 @@ class StartupExcessiveResourceUseTest { } } -private fun countAndLogViewHierarchyDepth(view: View, level: Int): Int { - // Log for debugging purposes: not sure if this is actually helpful. - val indent = "| ".repeat(level - 1) - Log.d("Startup...Test", "${indent}$view") - - return if (view !is ViewGroup) { - level - } else { - val maxDepth = view.children.map { countAndLogViewHierarchyDepth(it, level + 1) }.maxOrNull() - maxDepth ?: level - } -} - private fun countRecyclerViewConstraintLayoutChildren(view: View, parent: View?): Int { val viewValue = if (parent is RecyclerView && view is ConstraintLayout) { 1