Skip to content

Commit

Permalink
Refactor the API of cotent pool
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Andrew Wang authored and facebook-github-bot committed Jul 29, 2024
1 parent 9c83ea0 commit 5d580bb
Show file tree
Hide file tree
Showing 15 changed files with 40 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
* <p>For example:
*
* <pre><code>
*
* {@literal @OnCreateMountContentPool}
* static MountItemsPool.ItemPool OnCreateMountContentPool(int poolSizeOverride) {
* return new MountItemsPool.ItemPool(poolSizeOverride);
* }
* </code></pre>
*/
@Retention(RetentionPolicy.SOURCE)
public @interface OnCreateMountContentPool {}
15 changes: 7 additions & 8 deletions litho-core/src/main/java/com/facebook/litho/HostComponent.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -134,7 +133,7 @@ public boolean isEquivalentProps(@Nullable Component other, boolean shouldCompar

@Override
public int poolSize() {
return 45;
return ComponentsConfiguration.hostComponentPoolSize;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
*/
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ object MountSpecLifecycleTesterDrawableSpec {

@JvmStatic
@OnCreateMountContentPool
fun onCreateMountContentPool(): MountItemsPool.ItemPool = TrackingMountContentPool(1)
fun onCreateMountContentPool(poolSize: Int): MountItemsPool.ItemPool = TrackingMountContentPool(1)

@JvmStatic
@UiThread
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ object MountSpecLifecycleTesterSpec {

@JvmStatic
@OnCreateMountContentPool
fun onCreateMountContentPool(): MountItemsPool.ItemPool = TrackingMountContentPool(1)
fun onCreateMountContentPool(poolSize: Int): MountItemsPool.ItemPool = TrackingMountContentPool(1)

@JvmStatic
@UiThread
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ object MountSpecPureRenderLifecycleTesterSpec {

@JvmStatic
@OnCreateMountContentPool
fun onCreateMountContentPool(): MountItemsPool.ItemPool = TrackingMountContentPool(1)
fun onCreateMountContentPool(poolSize: Int): MountItemsPool.ItemPool = TrackingMountContentPool(1)

@JvmStatic
@UiThread
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions litho-it/src/test/resources/processor/TestMount.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions litho-it/src/test/resources/processor/TestMountSpec.java
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,8 @@ static boolean shouldUpdate(@Prop Diff<Integer> 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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ public final class DelegateMethodDescriptions {
.accessType(Modifier.PUBLIC)
.returnType(ClassNames.MOUNT_CONTENT_POOL)
.name("onCreateMountContentPool")
.definedParameterTypes(ImmutableList.<TypeName>of())
.definedParameterTypes(ImmutableList.<TypeName>of(TypeName.INT))
.optionalParameterTypes(ImmutableList.of(INJECT_PROP))
.build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,6 @@ interface ContentAllocator<Content : Any> {
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
Expand All @@ -71,7 +63,10 @@ interface ContentAllocator<Content : Any> {
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. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ object MountItemsPool {

if (pool == null) {
pool =
allocator.createRecyclingPool(poolSize)
allocator.onCreateMountContentPool(poolSize)
?: DefaultItemPool(poolableContentType, poolSize)
}

Expand Down

0 comments on commit 5d580bb

Please sign in to comment.