From 73afa391f8499c022c432407048b852d195c3238 Mon Sep 17 00:00:00 2001 From: Luke De Feo Date: Wed, 25 Jan 2023 04:47:11 -0800 Subject: [PATCH] Introduce shared throttle Summary: Even with batching, updates can get split in half due to fact that each litho view has its own independant throttle. Eventually they will drift and and a traversal will get scheduled past the current frame for some of the views. It results in artifacts in the visualiser and will make time travelling wonky The Reviewed By: lblasa Differential Revision: D42606932 fbshipit-source-id: c4cdf729302a380928b4d8720a59d5f7f6ff645a --- .../plugins/uidebugger/core/Context.kt | 3 +- .../observers/ApplicationTreeObserver.kt | 9 ++- .../observers/DecorViewTreeObserver.kt | 47 ++++++------ .../uidebugger/observers/TreeObserver.kt | 2 +- .../observers/TreeObserverManager.kt | 3 + .../uidebugger/scheduler/CoroutineThrottle.kt | 46 ------------ .../uidebugger/scheduler/SharedThrottle.kt | 74 +++++++++++++++++++ 7 files changed, 111 insertions(+), 73 deletions(-) delete mode 100644 android/src/main/java/com/facebook/flipper/plugins/uidebugger/scheduler/CoroutineThrottle.kt create mode 100644 android/src/main/java/com/facebook/flipper/plugins/uidebugger/scheduler/SharedThrottle.kt diff --git a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/core/Context.kt b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/core/Context.kt index e6d9b41f9..28a701bc8 100644 --- a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/core/Context.kt +++ b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/core/Context.kt @@ -12,6 +12,7 @@ import com.facebook.flipper.plugins.uidebugger.common.BitmapPool import com.facebook.flipper.plugins.uidebugger.descriptors.DescriptorRegister import com.facebook.flipper.plugins.uidebugger.observers.TreeObserverFactory import com.facebook.flipper.plugins.uidebugger.observers.TreeObserverManager +import com.facebook.flipper.plugins.uidebugger.scheduler.SharedThrottle import com.facebook.flipper.plugins.uidebugger.traversal.PartialLayoutTraversal data class Context( @@ -24,7 +25,7 @@ data class Context( PartialLayoutTraversal(descriptorRegister, observerFactory) val treeObserverManager = TreeObserverManager(this) - + val sharedThrottle: SharedThrottle = SharedThrottle() val bitmapPool = BitmapPool() } diff --git a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/observers/ApplicationTreeObserver.kt b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/observers/ApplicationTreeObserver.kt index dd298da8d..6c81b1f6b 100644 --- a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/observers/ApplicationTreeObserver.kt +++ b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/observers/ApplicationTreeObserver.kt @@ -13,6 +13,7 @@ import com.facebook.flipper.plugins.uidebugger.LogTag import com.facebook.flipper.plugins.uidebugger.core.ApplicationRef import com.facebook.flipper.plugins.uidebugger.core.Context import com.facebook.flipper.plugins.uidebugger.core.RootViewResolver +import com.facebook.flipper.plugins.uidebugger.util.objectIdentity /** * Responsible for observing the activity stack and managing the subscription to the top most @@ -35,9 +36,14 @@ class ApplicationTreeObserver(val context: Context) : TreeObserver) { Log.i(LogTag, "Root views updated, num ${rootViews.size}") - processUpdate(context, applicationRef) + context.sharedThrottle.trigger() } } + + context.sharedThrottle.registerCallback(this.objectIdentity()) { + traverseAndSend(context, applicationRef) + } + context.applicationRef.rootsResolver.attachListener(rootViewListener) // On subscribe, trigger a traversal on whatever roots we have rootViewListener.onRootViewsChanged(applicationRef.rootsResolver.rootViews()) @@ -48,5 +54,6 @@ class ApplicationTreeObserver(val context: Context) : TreeObserver() { - private val throttleTimeMs = 500L - private var nodeRef: WeakReference? = null - private var listener: ViewTreeObserver.OnPreDrawListener? = null + private var preDrawListener: ViewTreeObserver.OnPreDrawListener? = null override val type = "DecorView" - private val waitScope = CoroutineScope(Dispatchers.IO) - private val mainScope = CoroutineScope(Dispatchers.Main) - override fun subscribe(node: Any) { node as View nodeRef = WeakReference(node) Log.i(LogTag, "Subscribing to decor view changes") - val throttledUpdate = - throttleLatest?>(throttleTimeMs, waitScope, mainScope) { weakView -> - weakView?.get()?.let { view -> - var snapshotBitmap: BitmapPool.ReusableBitmap? = null - if (view.width > 0 && view.height > 0) { - snapshotBitmap = context.bitmapPool.getBitmap(node.width, node.height) - } - processUpdate(context, view, snapshotBitmap) - } - } + context.sharedThrottle.registerCallback(this.objectIdentity()) { + nodeRef?.get()?.let { traverseAndSendWithSnapshot() } + } - listener = + preDrawListener = ViewTreeObserver.OnPreDrawListener { - throttledUpdate(nodeRef) + context.sharedThrottle.trigger() true } - node.viewTreeObserver.addOnPreDrawListener(listener) + node.viewTreeObserver.addOnPreDrawListener(preDrawListener) // It can be the case that the DecorView the current observer owns has already // drawn. In this case, manually trigger an update. - throttledUpdate(nodeRef) + traverseAndSendWithSnapshot() + } + + private fun traverseAndSendWithSnapshot() { + nodeRef?.get()?.let { view -> + var snapshotBitmap: BitmapPool.ReusableBitmap? = null + if (view.width > 0 && view.height > 0) { + snapshotBitmap = context.bitmapPool.getBitmap(view.width, view.height) + } + traverseAndSend(context, view, snapshotBitmap) + } } override fun unsubscribe() { Log.i(LogTag, "Unsubscribing from decor view changes") - listener.let { + preDrawListener.let { nodeRef?.get()?.viewTreeObserver?.removeOnPreDrawListener(it) - listener = null + preDrawListener = null } + context.sharedThrottle.deregisterCallback(this.objectIdentity()) + nodeRef?.clear() nodeRef = null } } diff --git a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/observers/TreeObserver.kt b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/observers/TreeObserver.kt index f8aad5695..b1ab98226 100644 --- a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/observers/TreeObserver.kt +++ b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/observers/TreeObserver.kt @@ -39,7 +39,7 @@ abstract class TreeObserver { abstract fun unsubscribe() /** Traverses the layout hierarchy while managing any encountered child observers. */ - fun processUpdate( + fun traverseAndSend( context: Context, root: Any, snapshotBitmap: BitmapPool.ReusableBitmap? = null diff --git a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/observers/TreeObserverManager.kt b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/observers/TreeObserverManager.kt index c32edc374..bf8048755 100644 --- a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/observers/TreeObserverManager.kt +++ b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/observers/TreeObserverManager.kt @@ -93,6 +93,9 @@ class TreeObserverManager(val context: Context) { private fun sendBatchedUpdate(batchedUpdate: BatchedUpdate) { + Log.i( + LogTag, + "Got update from ${batchedUpdate.updates.size} observers at time ${batchedUpdate.frameTimeMs}") val onWorkerThread = System.currentTimeMillis() val nodes = batchedUpdate.updates.flatMap { it.deferredNodes.map { it.value() } } diff --git a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/scheduler/CoroutineThrottle.kt b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/scheduler/CoroutineThrottle.kt deleted file mode 100644 index 1c97ca96f..000000000 --- a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/scheduler/CoroutineThrottle.kt +++ /dev/null @@ -1,46 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -package com.facebook.flipper.plugins.uidebugger.scheduler - -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Job -import kotlinx.coroutines.delay -import kotlinx.coroutines.launch - -/** - * Throttle the execution of an executable for the specified interval. - * - * How does it work? - * - * The function `throttleLatest` returns a proxy for the given executable. This proxy captures the - * latest argument/param that was used on the last invocation. If the throttle job does not exist or - * has already completed, then create a new one. - * - * The job will wait on the waiting scope for the given amount of specified ms. Once it finishes - * waiting, then it will execute the given executable on the main scope with the latest captured - * param. - */ -fun throttleLatest( - intervalMs: Long, - waitScope: CoroutineScope, - mainScope: CoroutineScope, - executable: (T) -> Unit -): (T) -> Unit { - var throttleJob: Job? = null - var latestParam: T - return { param: T -> - latestParam = param - if (throttleJob == null || throttleJob?.isCompleted == true) { - throttleJob = - waitScope.launch { - delay(intervalMs) - mainScope.launch { executable(latestParam) } - } - } - } -} diff --git a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/scheduler/SharedThrottle.kt b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/scheduler/SharedThrottle.kt new file mode 100644 index 000000000..795f3b0d9 --- /dev/null +++ b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/scheduler/SharedThrottle.kt @@ -0,0 +1,74 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package com.facebook.flipper.plugins.uidebugger.scheduler + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch + +/** + * The class makes the following guarantees + * 1. All registered callbacks will be called on the same frame at the same time + * 2. The callbacks will never be called more often than the min interval + * 3. If it has been > min interval since the callbacks was last called we will call the callbacks + * immediately + * 4. If an event comes in within the min interval of the last firing we will schedule another + * firing at the next possible moment + * + * The reason we need this is because with an independent throttle per observer you end up with + * updates occurring across different frames, + * + * WARNING: Not thread safe, should only be called on main thread. Also It is important to + * deregister to avoid leaks + */ +class SharedThrottle( + private val executionScope: CoroutineScope = CoroutineScope(Dispatchers.Main) +) { + + private var job: Job? = null + private val callbacks = mutableMapOf Unit>() + private var latestInvocationId: Long = 0 + + fun registerCallback(id: Int, callback: () -> Unit) { + callbacks[id] = callback + } + + fun deregisterCallback(id: Int) { + callbacks.remove(id) + } + + fun trigger() { + latestInvocationId += 1 + + if (job == null || job?.isCompleted == true) { + job = + executionScope.launch { + var i = 0 + do { + val thisInvocationId = latestInvocationId + + callbacks.values.toList().forEach { callback -> callback() } + + val delayTime = exponentialBackOff(base = 250, exp = 1.5, max = 1000, i = i).toLong() + delay(delayTime) + i++ + + // if we haven't received an call since we executed break out and let a new job be + // created which, otherwise we loop which executes again at the next appropriate time + // since we have already waited + } while (thisInvocationId != latestInvocationId) + } + } + } + + private fun exponentialBackOff(base: Long, exp: Double, max: Long, i: Int): Double { + return Math.min(base * Math.pow(exp, i.toDouble()), max.toDouble()) + } +}