From 5d580bb0a0fe948b4242b231e76b44c01e0f49a7 Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Mon, 29 Jul 2024 05:36:35 -0700 Subject: [PATCH] Refactor the API of cotent pool Summary: ## Context Now we have two interface methods, `createRecyclingPool` and `onCreateMountContentPool`, which is confusing and caused wrong usages. This diff is to remove one of which and make `onCreateMountContentPool` the single truth of providing content pool. Reviewed By: adityasharat Differential Revision: D60367366 fbshipit-source-id: 78e2a91c306edb2ec44f0eba7a925b9aa7447754 --- .../annotations/OnCreateMountContentPool.java | 10 ++++++++++ .../java/com/facebook/litho/HostComponent.java | 15 +++++++-------- .../com/facebook/litho/HostMountContentPool.kt | 2 +- .../facebook/litho/MountSpecLithoRenderUnit.kt | 4 ++-- .../facebook/litho/SpecGeneratedComponent.java | 11 ++--------- .../litho/widget/CrashingMountableSpec.kt | 5 ++++- .../MountSpecLifecycleTesterDrawableSpec.kt | 2 +- .../litho/widget/MountSpecLifecycleTesterSpec.kt | 2 +- .../MountSpecPureRenderLifecycleTesterSpec.kt | 2 +- .../PreallocatedMountSpecLifecycleTesterSpec.kt | 2 +- .../src/test/resources/processor/TestMount.java | 4 ++-- .../test/resources/processor/TestMountSpec.java | 4 ++-- .../model/DelegateMethodDescriptions.java | 2 +- .../com/facebook/rendercore/ContentAllocator.kt | 13 ++++--------- .../com/facebook/rendercore/MountItemsPool.kt | 2 +- 15 files changed, 40 insertions(+), 40 deletions(-) diff --git a/litho-annotations/src/main/java/com/facebook/litho/annotations/OnCreateMountContentPool.java b/litho-annotations/src/main/java/com/facebook/litho/annotations/OnCreateMountContentPool.java index e69a5d9b391..f5b3099cc45 100644 --- a/litho-annotations/src/main/java/com/facebook/litho/annotations/OnCreateMountContentPool.java +++ b/litho-annotations/src/main/java/com/facebook/litho/annotations/OnCreateMountContentPool.java @@ -23,6 +23,16 @@ * The annotated method will be called to create a custom MountContentPool for this mount spec. If a * method with this annotation isn't provided, a DefaultMountContentPool will be used to recycle the * mount content of this mount spec, which is almost always sufficient. + * + *

For example: + * + * 


+ *
+ *  {@literal @OnCreateMountContentPool}
+ *   static MountItemsPool.ItemPool OnCreateMountContentPool(int poolSizeOverride) {
+ *     return new MountItemsPool.ItemPool(poolSizeOverride);
+ *   }
+ *
*/ @Retention(RetentionPolicy.SOURCE) public @interface OnCreateMountContentPool {} diff --git a/litho-core/src/main/java/com/facebook/litho/HostComponent.java b/litho-core/src/main/java/com/facebook/litho/HostComponent.java index 90c438504d2..c9f665ced56 100644 --- a/litho-core/src/main/java/com/facebook/litho/HostComponent.java +++ b/litho-core/src/main/java/com/facebook/litho/HostComponent.java @@ -46,13 +46,6 @@ protected HostComponent( mPoolingPolicy = poolingPolicy; } - @Override - public MountItemsPool.ItemPool onCreateMountContentPool() { - return new HostMountContentPool( - ComponentsConfiguration.hostComponentPoolSize, - mPoolingPolicy.canAcquireContent || mPoolingPolicy.canReleaseContent); - } - @Override public PoolingPolicy getPoolingPolicy() { return mPoolingPolicy; @@ -68,6 +61,12 @@ protected Object onCreateMountContent(Context c) { return new ComponentHost(c, mUnsafeModificationPolicy); } + @Override + public MountItemsPool.ItemPool onCreateMountContentPool(int poolSizeOverride) { + return new HostMountContentPool( + poolSizeOverride, mPoolingPolicy.canAcquireContent || mPoolingPolicy.canReleaseContent); + } + @Override protected void onMount( final @Nullable ComponentContext c, @@ -134,7 +133,7 @@ public boolean isEquivalentProps(@Nullable Component other, boolean shouldCompar @Override public int poolSize() { - return 45; + return ComponentsConfiguration.hostComponentPoolSize; } @Override diff --git a/litho-core/src/main/java/com/facebook/litho/HostMountContentPool.kt b/litho-core/src/main/java/com/facebook/litho/HostMountContentPool.kt index f8fde480959..0b69878df90 100644 --- a/litho-core/src/main/java/com/facebook/litho/HostMountContentPool.kt +++ b/litho-core/src/main/java/com/facebook/litho/HostMountContentPool.kt @@ -47,7 +47,7 @@ class HostMountContentPool(maxSize: Int, isEnabled: Boolean) : ItemPool { return false } - return pool?.release(item) ?: false + return pool.release(item) } override fun maybePreallocateContent(c: Context, contentAllocator: ContentAllocator<*>): Boolean = diff --git a/litho-core/src/main/java/com/facebook/litho/MountSpecLithoRenderUnit.kt b/litho-core/src/main/java/com/facebook/litho/MountSpecLithoRenderUnit.kt index 88f7224f159..ad0c50dfdc2 100644 --- a/litho-core/src/main/java/com/facebook/litho/MountSpecLithoRenderUnit.kt +++ b/litho-core/src/main/java/com/facebook/litho/MountSpecLithoRenderUnit.kt @@ -76,10 +76,10 @@ private constructor( isShouldUpdateResultCached = false } - override fun createRecyclingPool(poolSizeOverride: Int): ItemPool? { + override fun onCreateMountContentPool(poolSizeOverride: Int): ItemPool? { return try { if (component is SpecGeneratedComponent) { - component.createRecyclingPool() + component.onCreateMountContentPool(poolSizeOverride) } else { null } diff --git a/litho-core/src/main/java/com/facebook/litho/SpecGeneratedComponent.java b/litho-core/src/main/java/com/facebook/litho/SpecGeneratedComponent.java index d7446af8eab..d92dfdc2258 100644 --- a/litho-core/src/main/java/com/facebook/litho/SpecGeneratedComponent.java +++ b/litho-core/src/main/java/com/facebook/litho/SpecGeneratedComponent.java @@ -58,7 +58,6 @@ public abstract class SpecGeneratedComponent extends Component EventTriggerTarget, HasEventTrigger { - private static final int DEFAULT_MAX_PREALLOCATION = 3; private static final DynamicValue[] sEmptyArray = new DynamicValue[0]; private final String mSimpleName; @@ -817,12 +816,6 @@ public Class getPoolableContentType() { return getClass(); } - @Nullable - @Override - public MountItemsPool.ItemPool createRecyclingPool(int poolSize) { - return onCreateMountContentPool(); - } - /** * @return true if this component can be preallocated. */ @@ -835,8 +828,8 @@ public boolean canPreallocate() { * @return the MountContentPool that should be used to recycle mount content for this mount spec. */ @Override - public MountItemsPool.ItemPool onCreateMountContentPool() { - return new MountItemsPool.DefaultItemPool(getPoolableContentType(), poolSize()); + public MountItemsPool.ItemPool onCreateMountContentPool(int poolSizeOverride) { + return new MountItemsPool.DefaultItemPool(getPoolableContentType(), poolSizeOverride); } @ThreadSafe diff --git a/litho-it/src/main/java/com/facebook/litho/widget/CrashingMountableSpec.kt b/litho-it/src/main/java/com/facebook/litho/widget/CrashingMountableSpec.kt index 8efd52d774f..b2e43e6d483 100644 --- a/litho-it/src/main/java/com/facebook/litho/widget/CrashingMountableSpec.kt +++ b/litho-it/src/main/java/com/facebook/litho/widget/CrashingMountableSpec.kt @@ -134,7 +134,10 @@ object CrashingMountableSpec { @JvmStatic @OnCreateMountContentPool - fun onCreateMountContentPool(@InjectProp lifecycle: LifecycleStep): MountItemsPool.ItemPool { + fun onCreateMountContentPool( + poolSize: Int, + @InjectProp lifecycle: LifecycleStep + ): MountItemsPool.ItemPool { if (lifecycle == LifecycleStep.ON_CREATE_MOUNT_CONTENT_POOL) { throw MountPhaseException(LifecycleStep.ON_CREATE_MOUNT_CONTENT_POOL) } diff --git a/litho-it/src/main/java/com/facebook/litho/widget/MountSpecLifecycleTesterDrawableSpec.kt b/litho-it/src/main/java/com/facebook/litho/widget/MountSpecLifecycleTesterDrawableSpec.kt index b894bb74a41..f7a7842750c 100644 --- a/litho-it/src/main/java/com/facebook/litho/widget/MountSpecLifecycleTesterDrawableSpec.kt +++ b/litho-it/src/main/java/com/facebook/litho/widget/MountSpecLifecycleTesterDrawableSpec.kt @@ -115,7 +115,7 @@ object MountSpecLifecycleTesterDrawableSpec { @JvmStatic @OnCreateMountContentPool - fun onCreateMountContentPool(): MountItemsPool.ItemPool = TrackingMountContentPool(1) + fun onCreateMountContentPool(poolSize: Int): MountItemsPool.ItemPool = TrackingMountContentPool(1) @JvmStatic @UiThread diff --git a/litho-it/src/main/java/com/facebook/litho/widget/MountSpecLifecycleTesterSpec.kt b/litho-it/src/main/java/com/facebook/litho/widget/MountSpecLifecycleTesterSpec.kt index bb0eacb5a04..08b55af5de5 100644 --- a/litho-it/src/main/java/com/facebook/litho/widget/MountSpecLifecycleTesterSpec.kt +++ b/litho-it/src/main/java/com/facebook/litho/widget/MountSpecLifecycleTesterSpec.kt @@ -122,7 +122,7 @@ object MountSpecLifecycleTesterSpec { @JvmStatic @OnCreateMountContentPool - fun onCreateMountContentPool(): MountItemsPool.ItemPool = TrackingMountContentPool(1) + fun onCreateMountContentPool(poolSize: Int): MountItemsPool.ItemPool = TrackingMountContentPool(1) @JvmStatic @UiThread diff --git a/litho-it/src/main/java/com/facebook/litho/widget/MountSpecPureRenderLifecycleTesterSpec.kt b/litho-it/src/main/java/com/facebook/litho/widget/MountSpecPureRenderLifecycleTesterSpec.kt index 13c511b0767..874bfafcd56 100644 --- a/litho-it/src/main/java/com/facebook/litho/widget/MountSpecPureRenderLifecycleTesterSpec.kt +++ b/litho-it/src/main/java/com/facebook/litho/widget/MountSpecPureRenderLifecycleTesterSpec.kt @@ -120,7 +120,7 @@ object MountSpecPureRenderLifecycleTesterSpec { @JvmStatic @OnCreateMountContentPool - fun onCreateMountContentPool(): MountItemsPool.ItemPool = TrackingMountContentPool(1) + fun onCreateMountContentPool(poolSize: Int): MountItemsPool.ItemPool = TrackingMountContentPool(1) @JvmStatic @UiThread diff --git a/litho-it/src/main/java/com/facebook/litho/widget/PreallocatedMountSpecLifecycleTesterSpec.kt b/litho-it/src/main/java/com/facebook/litho/widget/PreallocatedMountSpecLifecycleTesterSpec.kt index 5dfdb92426e..dc3e8118ea7 100644 --- a/litho-it/src/main/java/com/facebook/litho/widget/PreallocatedMountSpecLifecycleTesterSpec.kt +++ b/litho-it/src/main/java/com/facebook/litho/widget/PreallocatedMountSpecLifecycleTesterSpec.kt @@ -85,7 +85,7 @@ object PreallocatedMountSpecLifecycleTesterSpec { @JvmStatic @UiThread @OnCreateMountContentPool - fun onCreateMountContentPool(): MountItemsPool.ItemPool = TrackingMountContentPool(1) + fun onCreateMountContentPool(poolSize: Int): MountItemsPool.ItemPool = TrackingMountContentPool(1) @JvmStatic @UiThread diff --git a/litho-it/src/test/resources/processor/TestMount.java b/litho-it/src/test/resources/processor/TestMount.java index 834fb8e5e3a..67db6768146 100644 --- a/litho-it/src/test/resources/processor/TestMount.java +++ b/litho-it/src/test/resources/processor/TestMount.java @@ -477,9 +477,9 @@ public boolean implementsShouldUpdate() { } @Override - public MountItemsPool.ItemPool onCreateMountContentPool() { + public MountItemsPool.ItemPool onCreateMountContentPool(int size) { MountItemsPool.ItemPool _result; - _result = (MountItemsPool.ItemPool) TestMountSpec.onCreateMountContentPool(); + _result = (MountItemsPool.ItemPool) TestMountSpec.onCreateMountContentPool(size); return _result; } diff --git a/litho-it/src/test/resources/processor/TestMountSpec.java b/litho-it/src/test/resources/processor/TestMountSpec.java index b5875bd20e1..76bdbdf4c8d 100644 --- a/litho-it/src/test/resources/processor/TestMountSpec.java +++ b/litho-it/src/test/resources/processor/TestMountSpec.java @@ -216,8 +216,8 @@ static boolean shouldUpdate(@Prop Diff prop1) { } @OnCreateMountContentPool - static MountItemsPool.ItemPool onCreateMountContentPool() { - return new MountItemsPool.DefaultItemPool(TestMountSpec.class, 3); + static MountItemsPool.ItemPool onCreateMountContentPool(int size) { + return new MountItemsPool.DefaultItemPool(TestMountSpec.class, size); } @OnCalculateCachedValue(name = "cached") diff --git a/litho-processor/src/main/java/com/facebook/litho/specmodels/model/DelegateMethodDescriptions.java b/litho-processor/src/main/java/com/facebook/litho/specmodels/model/DelegateMethodDescriptions.java index f99e66dc6fd..56ccc5099b2 100644 --- a/litho-processor/src/main/java/com/facebook/litho/specmodels/model/DelegateMethodDescriptions.java +++ b/litho-processor/src/main/java/com/facebook/litho/specmodels/model/DelegateMethodDescriptions.java @@ -258,7 +258,7 @@ public final class DelegateMethodDescriptions { .accessType(Modifier.PUBLIC) .returnType(ClassNames.MOUNT_CONTENT_POOL) .name("onCreateMountContentPool") - .definedParameterTypes(ImmutableList.of()) + .definedParameterTypes(ImmutableList.of(TypeName.INT)) .optionalParameterTypes(ImmutableList.of(INJECT_PROP)) .build(); diff --git a/litho-rendercore/src/main/java/com/facebook/rendercore/ContentAllocator.kt b/litho-rendercore/src/main/java/com/facebook/rendercore/ContentAllocator.kt index a8468aa5522..ad52ca9cffa 100644 --- a/litho-rendercore/src/main/java/com/facebook/rendercore/ContentAllocator.kt +++ b/litho-rendercore/src/main/java/com/facebook/rendercore/ContentAllocator.kt @@ -50,14 +50,6 @@ interface ContentAllocator { val poolingPolicy: PoolingPolicy get() = PoolingPolicy.Default - fun createRecyclingPool(poolSizeOverride: Int = UNSET_POOL_SIZE): ItemPool? { - return if (poolSizeOverride > UNSET_POOL_SIZE) { - DefaultItemPool(javaClass, poolSizeOverride) - } else { - onCreateMountContentPool() - } - } - /** * This API informs the framework to fill the content pool for this Mountable ahead of time. The * default value is `false`, i.e. content is not pre-allocated. Pre-allocation of the content can @@ -71,7 +63,10 @@ interface ContentAllocator { fun poolSize(): Int = DEFAULT_MAX_PREALLOCATION /** Creates the content pool the framework should use for this [ContentAllocator] */ - fun onCreateMountContentPool(): ItemPool = DefaultItemPool(javaClass, poolSize()) + fun onCreateMountContentPool(poolSizeOverride: Int = UNSET_POOL_SIZE): ItemPool? { + val size = if (poolSizeOverride > UNSET_POOL_SIZE) poolSizeOverride else poolSize() + return DefaultItemPool(javaClass, size) + } companion object { /** Default size of the content pool. */ diff --git a/litho-rendercore/src/main/java/com/facebook/rendercore/MountItemsPool.kt b/litho-rendercore/src/main/java/com/facebook/rendercore/MountItemsPool.kt index ac1349a1227..35cd3d61f53 100644 --- a/litho-rendercore/src/main/java/com/facebook/rendercore/MountItemsPool.kt +++ b/litho-rendercore/src/main/java/com/facebook/rendercore/MountItemsPool.kt @@ -203,7 +203,7 @@ object MountItemsPool { if (pool == null) { pool = - allocator.createRecyclingPool(poolSize) + allocator.onCreateMountContentPool(poolSize) ?: DefaultItemPool(poolableContentType, poolSize) }