Skip to content

Commit

Permalink
Allow manual viewport estimation to avoid sync initRange calls
Browse files Browse the repository at this point in the history 
Summary:
The goal here is to have a manual bypass around sync initRange calls on the main thread: we do this by specifying a manual viewport range so that a range doesn't have to be determined before kicking off async layouts.

Will follow up with Mihaela on longer term mitigation here (there are already params in flight).

Reviewed By: muraziz, paveldudka

Differential Revision: D16599558

fbshipit-source-id: 018a15a2aa986f5aef4235529c2a76990ab37c13
  • Loading branch information
@astreet @facebook-github-bot
astreet authored and facebook-github-bot committed Aug 2, 2019
1 parent b3dfc1b commit 3308ee8
Show file tree
Hide file tree
Showing 3 changed files with 232 additions and 4 deletions.
189 changes: 189 additions & 0 deletions litho-it/src/test/java/com/facebook/litho/widget/RecyclerBinderManualRangeTest.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
/*
* Copyright 2014-present Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.facebook.litho.widget;

import static com.facebook.litho.SizeSpec.EXACTLY;
import static com.facebook.litho.SizeSpec.UNSPECIFIED;
import static com.facebook.litho.SizeSpec.makeSizeSpec;
import static com.facebook.litho.widget.ComponentRenderInfo.create;
import static com.facebook.litho.widget.RecyclerBinderTest.NO_OP_CHANGE_SET_COMPLETE_CALLBACK;
import static org.assertj.core.api.Java6Assertions.assertThat;
import static org.mockito.Mockito.mock;

import android.os.Looper;
import com.facebook.litho.ComponentContext;
import com.facebook.litho.ComponentTree;
import com.facebook.litho.EventHandler;
import com.facebook.litho.Size;
import com.facebook.litho.SizeSpec;
import com.facebook.litho.testing.TestDrawableComponent;
import com.facebook.litho.testing.Whitebox;
import com.facebook.litho.testing.testrunner.ComponentsTestRunner;
import java.util.ArrayList;
import java.util.List;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.junit.runner.RunWith;
import org.robolectric.RuntimeEnvironment;
import org.robolectric.Shadows;
import org.robolectric.shadows.ShadowLooper;

/** Tests for manually specifying estimatedViewportCount to {@link RecyclerBinder} */
@RunWith(ComponentsTestRunner.class)
public class RecyclerBinderManualRangeTest {

@Rule public ExpectedException mExpectedException = ExpectedException.none();

private ComponentContext mComponentContext;
private ShadowLooper mLayoutThreadShadowLooper;

@Before
public void setup() {
mComponentContext = new ComponentContext(RuntimeEnvironment.application);
mComponentContext.getAndroidContext().setTheme(0);

mLayoutThreadShadowLooper =
Shadows.shadowOf(
(Looper) Whitebox.invokeMethod(ComponentTree.class, "getDefaultLayoutThreadLooper"));
}

@After
public void tearDown() {
mLayoutThreadShadowLooper.runToEndOfTasks();
}

@Test
public void testMeasureAfterAddItems() {
final int widthSpec = SizeSpec.makeSizeSpec(200, SizeSpec.EXACTLY);
final int heightSpec = SizeSpec.makeSizeSpec(200, SizeSpec.EXACTLY);

final RecyclerBinder recyclerBinder =
new RecyclerBinder.Builder()
.estimatedViewportCount(1)
.rangeRatio(.5f)
.build(mComponentContext);

final List<RenderInfo> initialComponents = new ArrayList<>();
for (int i = 0; i < 10; i++) {
initialComponents.add(
create()
.component(
TestDrawableComponent.create(mComponentContext)
.widthPx(100)
.heightPx(100)
.build())
.build());
}
recyclerBinder.insertRangeAt(0, initialComponents);
recyclerBinder.notifyChangeSetComplete(true, NO_OP_CHANGE_SET_COMPLETE_CALLBACK);

recyclerBinder.measure(new Size(), widthSpec, heightSpec, null);

for (int i = 0; i < initialComponents.size(); i++) {
final ComponentTreeHolder holder = recyclerBinder.getComponentTreeHolderAt(i);
assertThat(holder.hasCompletedLatestLayout()).describedAs("Holder " + i).isFalse();
}

mLayoutThreadShadowLooper.runToEndOfTasks();

for (int i = 0; i < 2; i++) {
final ComponentTreeHolder holder = recyclerBinder.getComponentTreeHolderAt(i);
assertThat(holder.hasCompletedLatestLayout()).describedAs("Holder " + i).isTrue();
assertThat(holder.isTreeValid()).describedAs("Holder " + i).isTrue();
RecyclerBinderTest.assertHasCompatibleLayout(
recyclerBinder, i, makeSizeSpec(200, EXACTLY), makeSizeSpec(0, UNSPECIFIED));
}

for (int i = 2; i < initialComponents.size(); i++) {
final ComponentTreeHolder holder = recyclerBinder.getComponentTreeHolderAt(i);
assertThat(holder.hasCompletedLatestLayout()).describedAs("Holder " + i).isFalse();
assertThat(holder.isTreeValid()).describedAs("Holder " + i).isFalse();
}
}

@Test
public void testAddItemsAfterMeasure() {
final int widthSpec = SizeSpec.makeSizeSpec(200, SizeSpec.EXACTLY);
final int heightSpec = SizeSpec.makeSizeSpec(200, SizeSpec.EXACTLY);

final RecyclerBinder recyclerBinder =
new RecyclerBinder.Builder()
.estimatedViewportCount(1)
.rangeRatio(.5f)
.build(mComponentContext);

recyclerBinder.measure(new Size(), widthSpec, heightSpec, null);

final List<RenderInfo> initialComponents = new ArrayList<>();
for (int i = 0; i < 10; i++) {
initialComponents.add(
create()
.component(
TestDrawableComponent.create(mComponentContext)
.widthPx(100)
.heightPx(100)
.build())
.build());
}
recyclerBinder.insertRangeAt(0, initialComponents);
recyclerBinder.notifyChangeSetComplete(true, NO_OP_CHANGE_SET_COMPLETE_CALLBACK);

for (int i = 0; i < initialComponents.size(); i++) {
final ComponentTreeHolder holder = recyclerBinder.getComponentTreeHolderAt(i);
assertThat(holder.hasCompletedLatestLayout()).describedAs("Holder " + i).isFalse();
}

mLayoutThreadShadowLooper.runToEndOfTasks();

for (int i = 0; i < 2; i++) {
final ComponentTreeHolder holder = recyclerBinder.getComponentTreeHolderAt(i);
assertThat(holder.hasCompletedLatestLayout()).describedAs("Holder " + i).isTrue();
assertThat(holder.isTreeValid()).describedAs("Holder " + i).isTrue();
RecyclerBinderTest.assertHasCompatibleLayout(
recyclerBinder, i, makeSizeSpec(200, EXACTLY), makeSizeSpec(0, UNSPECIFIED));
}

for (int i = 2; i < initialComponents.size(); i++) {
final ComponentTreeHolder holder = recyclerBinder.getComponentTreeHolderAt(i);
assertThat(holder.hasCompletedLatestLayout()).describedAs("Holder " + i).isFalse();
assertThat(holder.isTreeValid()).describedAs("Holder " + i).isFalse();
}
}

@Test
public void testCanMeasureIsUnsupported() {
mExpectedException.expect(RuntimeException.class);
mExpectedException.expectMessage(
"Cannot use manual estimated viewport count when the RecyclerBinder needs an item to determine its size!");

final RecyclerBinder recyclerBinder =
new RecyclerBinder.Builder()
.estimatedViewportCount(1)
.rangeRatio(.5f)
.canMeasure(true)
.build(mComponentContext);

recyclerBinder.measure(
new Size(),
SizeSpec.makeSizeSpec(200, UNSPECIFIED),
SizeSpec.makeSizeSpec(200, SizeSpec.EXACTLY),
mock(EventHandler.class));
}
}
2 changes: 1 addition & 1 deletion litho-it/src/test/java/com/facebook/litho/widget/RecyclerBinderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5312,7 +5312,7 @@ private RenderInfo createTestComponentRenderInfo() {
.build();
}

private void assertHasCompatibleLayout(
static void assertHasCompatibleLayout(
RecyclerBinder recyclerBinder, int position, int widthSpec, int heightSpec) {
final ComponentTree tree = recyclerBinder.getComponentAt(position);
assertThat(tree).isNotNull();
Expand Down
45 changes: 42 additions & 3 deletions litho-widget/src/main/java/com/facebook/litho/widget/RecyclerBinder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ public class RecyclerBinder
private final boolean mUseCancelableLayoutFutures;
private final boolean mMoveLayoutsBetweenThreads;
private final boolean mIsSubAdapter;
private final boolean mHasManualEstimatedViewportCount;

private AtomicLong mCurrentChangeSetThreadId = new AtomicLong(-1);
@VisibleForTesting final boolean mTraverseLayoutBackwards;
Expand Down Expand Up @@ -398,6 +399,7 @@ public static class Builder {
private boolean canInterruptAndMoveLayoutsBetweenThreads =
ComponentsConfiguration.canInterruptAndMoveLayoutsBetweenThreads;
private boolean isSubAdapter;
private int estimatedViewportCount = UNSET;

/**
* @param rangeRatio specifies how big a range this binder should try to compute. The range is
Expand Down Expand Up @@ -571,6 +573,20 @@ public Builder bgScheduleAllInitRange(boolean bgScheduleAllInitRange) {
return this;
}

/**
* This is a temporary hack that allows a surface to manually provide an estimated range. It
* will go away so don't depend on it.
*/
@Deprecated
public Builder estimatedViewportCount(int estimatedViewportCount) {
if (estimatedViewportCount <= 0) {
throw new IllegalArgumentException(
"Estimated viewport count must be > 0: " + estimatedViewportCount);
}
this.estimatedViewportCount = estimatedViewportCount;
return this;
}

/**
* Only for horizontally scrolling layouts! If true, the height of the RecyclerView is not known
* when it's measured; the first item is measured and its height will determine the height of
Expand Down Expand Up @@ -846,6 +862,13 @@ private RecyclerBinder(Builder builder) {

mInvalidStateLogParamsList = builder.invalidStateLogParamsList;

if (builder.estimatedViewportCount != UNSET) {
mEstimatedViewportCount = builder.estimatedViewportCount;
mHasManualEstimatedViewportCount = true;
} else {
mHasManualEstimatedViewportCount = false;
}

mAsyncInitRange = builder.asyncInitRange;
mBgScheduleAllInitRange = builder.bgScheduleAllInitRange;
mHScrollAsyncMode = builder.hscrollAsyncMode;
Expand Down Expand Up @@ -1991,6 +2014,11 @@ public void measure(

final boolean shouldMeasureItemForSize =
shouldMeasureItemForSize(widthSpec, heightSpec, scrollDirection, canRemeasure);
if (mHasManualEstimatedViewportCount && shouldMeasureItemForSize) {
throw new RuntimeException(
"Cannot use manual estimated viewport count when the RecyclerBinder needs an item to determine its size!");
}

ComponentTreeHolderRangeInfo holderForRangeInfo;

mIsInMeasure.set(true);
Expand Down Expand Up @@ -2400,7 +2428,10 @@ private void assertNoRemoveOperationIfCircular(int removeCount) {

@GuardedBy("this")
private void invalidateLayoutData() {
mEstimatedViewportCount = UNSET;
if (!mHasManualEstimatedViewportCount) {
mEstimatedViewportCount = UNSET;
}

mSizeForMeasure = null;
for (int i = 0, size = mComponentTreeHolders.size(); i < size; i++) {
mComponentTreeHolders.get(i).invalidateTree();
Expand Down Expand Up @@ -2482,6 +2513,9 @@ public void setAsyncInitRange(boolean asyncInitRange) {
@GuardedBy("this")
void initRange(
int width, int height, ComponentTreeHolderRangeInfo holderRangeInfo, int scrollDirection) {
if (mHasManualEstimatedViewportCount) {
return;
}

if (asyncInitRangeEnabled()) {
// We can schedule a maximum of number of items minus one (which is being calculated
Expand Down Expand Up @@ -2581,6 +2615,10 @@ public void onSetRootAndSizeSpec(int itemWidth, int itemHeight) {
}

private void setRangeSize(int itemWidth, int itemHeight, int width, int height) {
if (mHasManualEstimatedViewportCount) {
throw new RuntimeException(
"Cannot override range size when manual estimated viewport count is set");
}
mEstimatedViewportCount =
Math.max(mLayoutInfo.approximateRangeSize(itemWidth, itemHeight, width, height), 1);
}
Expand Down Expand Up @@ -2610,7 +2648,7 @@ private void layoutItemForSize(ComponentTreeHolderRangeInfo holderRangeInfo) {
@GuardedBy("this")
private void resetMeasuredSize(int width) {
// we will set a range anyway if it's null, no need to do this now.
if (mSizeForMeasure == null) {
if (mSizeForMeasure == null || mHasManualEstimatedViewportCount) {
return;
}
int maxHeight = 0;
Expand Down Expand Up @@ -3148,7 +3186,8 @@ RangeCalculationResult getRangeCalculationResult() {
}

private boolean hasComputedRange() {
return mSizeForMeasure != null && mEstimatedViewportCount != UNSET;
return (mSizeForMeasure != null && mEstimatedViewportCount != UNSET)
|| mHasManualEstimatedViewportCount;
}

/**
Expand Down

0 comments on commit 3308ee8

Please sign in to comment.