Skip to content

Commit

Permalink
For mozilla-mobile#21183: remove view hierarchy depth check from exce…
Browse files Browse the repository at this point in the history
…ssive 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.
  • Loading branch information
mcomella authored and mergify[bot] committed Sep 8, 2021
1 parent 4fe6a45 commit d4b13ce
Showing 1 changed file with 0 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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 " +
Expand Down Expand Up @@ -89,14 +81,12 @@ class StartupExcessiveResourceUseTest {
val actualRunBlocking = RunBlockingCounter.count.get()

val rootView = activityTestRule.activity.findViewById<LinearLayout>(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,
Expand All @@ -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
Expand Down

0 comments on commit d4b13ce

Please sign in to comment.