From 04a99a1dc8d7d472a3820aafac47d5ba16ce5035 Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Tue, 10 Dec 2024 05:09:27 -0800 Subject: [PATCH] Improve logging for resolving without size Summary: As per title Reviewed By: adityasharat Differential Revision: D66957544 fbshipit-source-id: 093bddffe29b823ba32e30cf3ed80fcf6c1631ce --- .../com/facebook/litho/ComponentTree.java | 81 ++++++++++--------- .../java/com/facebook/litho/TreeFuture.kt | 3 +- .../litho/config/ComponentsConfiguration.kt | 8 ++ 3 files changed, 55 insertions(+), 37 deletions(-) diff --git a/litho-core/src/main/java/com/facebook/litho/ComponentTree.java b/litho-core/src/main/java/com/facebook/litho/ComponentTree.java index d5aa45901b7..f731c5cf636 100644 --- a/litho-core/src/main/java/com/facebook/litho/ComponentTree.java +++ b/litho-core/src/main/java/com/facebook/litho/ComponentTree.java @@ -1975,49 +1975,55 @@ private void doResolve( final @Nullable ResolveResult resolveResult = resolveResultHolder.result; if (resolveResult == null) { - if (getLithoConfiguration().componentsConfig.enableResolveWithoutSizeSpec) { + final boolean isLoggingEnabled = + getLithoConfiguration().componentsConfig.enableLoggingForRenderInFlight; + final boolean enableResolveWithoutSizeSpec = + getLithoConfiguration().componentsConfig.enableResolveWithoutSizeSpec; + final boolean enableFixForResolveWithoutSizeSpec = + getLithoConfiguration().componentsConfig.enableFixForResolveWithoutSizeSpec; + + if (isLoggingEnabled || enableResolveWithoutSizeSpec || enableFixForResolveWithoutSizeSpec) { final boolean isWaitingButInterrupted = - (resolveResultHolder.type == TreeFuture.FutureExecutionType.REUSE_FUTURE) - && (TreeFuture.FutureState.INTERRUPTED == resolveResultHolder.state); + TreeFuture.FutureState.WAITING == resolveResultHolder.state; final boolean isLatestRequest; synchronized (this) { // To check if this is the latest resolve request and we don't want to get lost of it. isLatestRequest = (localResolveVersion == (mNextResolveVersion - 1)); } - if (isWaitingButInterrupted && !ThreadUtils.isMainThread() && isLatestRequest) { - // If we found any interrupted async resolve task which is also the latest request, which - // means the current resolve request might be getting lost and we need to re-try it to - // make sure there's not render in flight. - requestRenderWithSplitFutures( - true, - output, - source, - extraAttribution, - widthSpec, - heightSpec, - root, - treePropContainer); - } - DebugInfoReporter.report( - "RenderInFlight:Null Result", - LogLevel.ERROR, - mId, - attributes -> { - attributes.put(DebugEventAttribute.Version, localResolveVersion); - attributes.put(DebugEventAttribute.Source, layoutSourceToString(source)); - attributes.put("Root", root.getSimpleName()); - attributes.put(DebugEventAttribute.Width, SizeSpec.toSimpleString(widthSpec)); - attributes.put(DebugEventAttribute.Height, SizeSpec.toSimpleString(heightSpec)); - attributes.put( - "withoutSizeSpec", - getLithoConfiguration().componentsConfig.enableResolveWithoutSizeSpec); - attributes.put("isLatestRequest", isLatestRequest); - attributes.put("isMainThread", ThreadUtils.isMainThread()); - attributes.put("FutureExecutionType", resolveResultHolder.type); - attributes.put("FutureState", resolveResultHolder.state); - return Unit.INSTANCE; - }); + if (isWaitingButInterrupted && isLatestRequest) { + if (enableFixForResolveWithoutSizeSpec) { + // If we found any interrupted async resolve task which is also the latest request, + // which + // means the current resolve request might be getting lost and we need to re-try it to + // make sure there's not render in flight. + requestRenderWithSplitFutures( + true, + output, + source, + extraAttribution, + widthSpec, + heightSpec, + root, + treePropContainer); + } + DebugInfoReporter.report( + "RenderInFlight v3:Null Result", + LogLevel.ERROR, + mId, + attributes -> { + attributes.put(DebugEventAttribute.Version, localResolveVersion); + attributes.put(DebugEventAttribute.Source, layoutSourceToString(source)); + attributes.put("Root", root.getSimpleName()); + attributes.put(DebugEventAttribute.Width, SizeSpec.toSimpleString(widthSpec)); + attributes.put(DebugEventAttribute.Height, SizeSpec.toSimpleString(heightSpec)); + attributes.put("withoutSizeSpec", enableResolveWithoutSizeSpec); + attributes.put("fix", enableFixForResolveWithoutSizeSpec); + attributes.put("FutureExecutionType", resolveResultHolder.type); + attributes.put("FutureState", resolveResultHolder.state); + return Unit.INSTANCE; + }); + } } } else { commitResolveResult(resolveResult); @@ -2249,6 +2255,9 @@ public void doFrame(long frameTimeNanos) { attributes.put( "withoutSizeSpec", getLithoConfiguration().componentsConfig.enableResolveWithoutSizeSpec); + attributes.put( + "fix", + getLithoConfiguration().componentsConfig.enableFixForResolveWithoutSizeSpec); attributes.put( "isRootNotCompatibleAndWithoutResolveFuture", isRootNotCompatibleAndWithoutResolveFuture); diff --git a/litho-core/src/main/java/com/facebook/litho/TreeFuture.kt b/litho-core/src/main/java/com/facebook/litho/TreeFuture.kt index b71c669f70a..134a2b179dd 100644 --- a/litho-core/src/main/java/com/facebook/litho/TreeFuture.kt +++ b/litho-core/src/main/java/com/facebook/litho/TreeFuture.kt @@ -275,7 +275,7 @@ abstract class TreeFuture( if (shouldWaitForResult && !isMainThread && !isFromSyncLayout(source)) { return TreeFutureResult.interruptWithMessage( type = type, - state = FutureState.INTERRUPTED, + state = FutureState.WAITING, description = FUTURE_RESULT_NULL_REASON_SYNC_RESULT_NON_MAIN_THREAD) } if (isMainThread && shouldWaitForResult) { @@ -456,6 +456,7 @@ abstract class TreeFuture( enum class FutureState { SUCCESS, + WAITING, INTERRUPTED, RELEASED } diff --git a/litho-core/src/main/java/com/facebook/litho/config/ComponentsConfiguration.kt b/litho-core/src/main/java/com/facebook/litho/config/ComponentsConfiguration.kt index 366e76be7cc..87819f1022e 100644 --- a/litho-core/src/main/java/com/facebook/litho/config/ComponentsConfiguration.kt +++ b/litho-core/src/main/java/com/facebook/litho/config/ComponentsConfiguration.kt @@ -128,6 +128,8 @@ internal constructor( * possibility of reusing resolve result. */ @JvmField val enableResolveWithoutSizeSpec: Boolean = false, + /** This will enable the fix for render in flight */ + @JvmField val enableFixForResolveWithoutSizeSpec: Boolean = false, /** This will skip calling `onDraw` for ComponentHost. */ @JvmField val enableHostWillNotDraw: Boolean = false, /** This will enable logging for render in-flight */ @@ -329,6 +331,7 @@ internal constructor( private var enableResolveWithoutSizeSpec = baseConfig.enableResolveWithoutSizeSpec private var enableHostWillNotDraw = baseConfig.enableHostWillNotDraw private var enableLoggingForRenderInFlight = baseConfig.enableLoggingForRenderInFlight + private var enableFixForResolveWithoutSizeSpec = baseConfig.enableFixForResolveWithoutSizeSpec private var enableFixForCachedNestedTree = baseConfig.enableFixForCachedNestedTree private var isHostViewAttributesCleanUpEnabled = baseConfig.isHostViewAttributesCleanUpEnabled @@ -430,6 +433,10 @@ internal constructor( enableLoggingForRenderInFlight = enabled } + fun enableFixForResolveWithoutSizeSpec(enabled: Boolean): Builder = also { + enableFixForResolveWithoutSizeSpec = enabled + } + fun enableFixForCachedNestedTree(enabled: Boolean): Builder = also { enableFixForCachedNestedTree = enabled } @@ -472,6 +479,7 @@ internal constructor( enableResolveWithoutSizeSpec = enableResolveWithoutSizeSpec, enableHostWillNotDraw = enableHostWillNotDraw, enableLoggingForRenderInFlight = enableLoggingForRenderInFlight, + enableFixForResolveWithoutSizeSpec = enableFixForResolveWithoutSizeSpec, enableFixForCachedNestedTree = enableFixForCachedNestedTree, isHostViewAttributesCleanUpEnabled = isHostViewAttributesCleanUpEnabled, )