From 6e5b4b3ce68ab7df0b16f1b09656944ed2a7c5ba Mon Sep 17 00:00:00 2001 From: Michael Comella Date: Wed, 31 Mar 2021 09:30:18 -0700 Subject: [PATCH] For #18731: remove anonymous classes from StrictModeManager. After this change, I took 3 profiles: the new code appeared in the profiler only once and only for one sample (i.e. possibly just got unlucky). It seems to be improved. Profiles: - https://share.firefox.dev/3wifiV2 (captured it) - https://share.firefox.dev/39xgdHz - https://share.firefox.dev/2QMdlA0 --- .../mozilla/fenix/perf/StrictModeManager.kt | 36 +++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/perf/StrictModeManager.kt b/app/src/main/java/org/mozilla/fenix/perf/StrictModeManager.kt index 845076d8c..0878a2651 100644 --- a/app/src/main/java/org/mozilla/fenix/perf/StrictModeManager.kt +++ b/app/src/main/java/org/mozilla/fenix/perf/StrictModeManager.kt @@ -95,21 +95,7 @@ class StrictModeManager( * specific fragment. */ fun attachListenerToDisablePenaltyDeath(fragmentManager: FragmentManager) { - fragmentManager.registerFragmentLifecycleCallbacks(object : - FragmentManager.FragmentLifecycleCallbacks() { - override fun onFragmentResumed(fm: FragmentManager, f: Fragment) { - fm.unregisterFragmentLifecycleCallbacks(this) - - // If we don't post when using penaltyListener on P+, the violation listener is never - // called. My best guess is that, unlike penaltyDeath, the violations are not - // delivered instantaneously so posting gives time for the violation listeners to - // run before they are removed here. This may be a race so we give the listeners a - // little extra time to run too though this way we may accidentally trigger - // violations for non-startup, which we haven't planned to do yet. - Handler(mainLooper).postDelayed({ - enableStrictMode(setPenaltyDeath = false) - }, DELAY_TO_REMOVE_STRICT_MODE_MILLIS) - } }, false) + fragmentManager.registerFragmentLifecycleCallbacks(DisableStrictModeFragmentLifecycleCallbacks(), false) } /** @@ -147,6 +133,26 @@ class StrictModeManager( functionBlock() } } + + // If we use anonymous classes/functions in this class, we get a class load error with a slight perf impact. #18731 + inner class DisableStrictModeFragmentLifecycleCallbacks : FragmentManager.FragmentLifecycleCallbacks() { + override fun onFragmentResumed(fm: FragmentManager, f: Fragment) { + fm.unregisterFragmentLifecycleCallbacks(this) + + // If we don't post when using penaltyListener on P+, the violation listener is never + // called. My best guess is that, unlike penaltyDeath, the violations are not + // delivered instantaneously so posting gives time for the violation listeners to + // run before they are removed here. This may be a race so we give the listeners a + // little extra time to run too though this way we may accidentally trigger + // violations for non-startup, which we haven't planned to do yet. + Handler(Looper.getMainLooper()).postDelayed(::disableStrictMode, DELAY_TO_REMOVE_STRICT_MODE_MILLIS) + } + + // See comment on anonymous functions above. + private fun disableStrictMode() { + enableStrictMode(setPenaltyDeath = false) + } + } } /**