Skip to content

Commit

Permalink
prepare ResourceResolver to be shared across all Component$Builder in…
Browse files Browse the repository at this point in the history 
…stances

Summary: Local profiling has shown us that calling Context#getResources can be surprisingly expensive. We currently do this during every Component$Builder constructor. To improve perf, we want to instead do this once per Context. These changes make ResourceResolver ready to be shared that way - namely, we stop recycling/releasing ResourceResolver and we stop sharing the themeAttrs array across method calls since multiple threads might want to use it.

Reviewed By: astreet

Differential Revision: D16587479

fbshipit-source-id: 47b75cd4911cb5d9a0eebd3d1b79f3a382f3fe2f
  • Loading branch information
@usikder @facebook-github-bot
usikder authored and facebook-github-bot committed Aug 2, 2019
1 parent fa87c02 commit c93517c
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 62 deletions.
6 changes: 2 additions & 4 deletions litho-core/src/main/java/com/facebook/litho/Border.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,8 @@ public class Border {

@Retention(RetentionPolicy.SOURCE)
@IntDef(
flag = true,
value = {Corner.TOP_LEFT, Corner.TOP_RIGHT, Corner.BOTTOM_RIGHT, Corner.BOTTOM_LEFT}
)
flag = true,
value = {Corner.TOP_LEFT, Corner.TOP_RIGHT, Corner.BOTTOM_RIGHT, Corner.BOTTOM_LEFT})
public @interface Corner {
int TOP_LEFT = 0;
int TOP_RIGHT = 1;
Expand Down Expand Up @@ -446,7 +445,6 @@ public Builder pathDashEffect(

public Border build() {
checkNotBuilt();
mResourceResolver.release();
mResourceResolver = null;

if (mNumPathEffects == MAX_PATH_EFFECTS) {
Expand Down
54 changes: 14 additions & 40 deletions litho-core/src/main/java/com/facebook/litho/ResourceResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,11 @@
import androidx.annotation.StringRes;

public class ResourceResolver {
private Resources mResources;
private Resources.Theme mTheme;
private ResourceCache mResourceCache;

// Use for *Attr methods to retrieve attributes without needing to
// allocate a new int[] for each call.
private final int[] mAttrs = new int[1];
private final Resources mResources;
private final Resources.Theme mTheme;
private final ResourceCache mResourceCache;

public ResourceResolver(ComponentContext context) {
// Only temporary whilst conversion is underway.
init(context);
}

public void init(ComponentContext context) {
mResources = context.getAndroidContext().getResources();
mTheme = context.getAndroidContext().getTheme();
mResourceCache = context.getResourceCache();
Expand Down Expand Up @@ -230,8 +221,7 @@ public Drawable resolveDrawableRes(@DrawableRes int resId) {
}

public String resolveStringAttr(@AttrRes int attrResId, @StringRes int defResId) {
mAttrs[0] = attrResId;
TypedArray a = mTheme.obtainStyledAttributes(mAttrs);
TypedArray a = mTheme.obtainStyledAttributes(new int[] {attrResId});

try {
String result = a.getString(0);
Expand All @@ -247,8 +237,7 @@ public String resolveStringAttr(@AttrRes int attrResId, @StringRes int defResId)

@Nullable
public String[] resolveStringArrayAttr(@AttrRes int attrResId, @ArrayRes int defResId) {
mAttrs[0] = attrResId;
TypedArray a = mTheme.obtainStyledAttributes(mAttrs);
TypedArray a = mTheme.obtainStyledAttributes(new int[] {attrResId});

try {
return resolveStringArrayRes(a.getResourceId(0, defResId));
Expand All @@ -258,8 +247,7 @@ public String[] resolveStringArrayAttr(@AttrRes int attrResId, @ArrayRes int def
}

public int resolveIntAttr(@AttrRes int attrResId, @IntegerRes int defResId) {
mAttrs[0] = attrResId;
TypedArray a = mTheme.obtainStyledAttributes(mAttrs);
TypedArray a = mTheme.obtainStyledAttributes(new int[] {attrResId});

try {
return a.getInt(0, resolveIntRes(defResId));
Expand All @@ -270,8 +258,7 @@ public int resolveIntAttr(@AttrRes int attrResId, @IntegerRes int defResId) {

@Nullable
public int[] resolveIntArrayAttr(@AttrRes int attrResId, @ArrayRes int defResId) {
mAttrs[0] = attrResId;
TypedArray a = mTheme.obtainStyledAttributes(mAttrs);
TypedArray a = mTheme.obtainStyledAttributes(new int[] {attrResId});

try {
return resolveIntArrayRes(a.getResourceId(0, defResId));
Expand All @@ -292,8 +279,7 @@ public Integer[] resolveIntegerArrayAttr(@AttrRes int attrResId, @ArrayRes int d
}

public boolean resolveBoolAttr(@AttrRes int attrResId, @BoolRes int defResId) {
mAttrs[0] = attrResId;
TypedArray a = mTheme.obtainStyledAttributes(mAttrs);
TypedArray a = mTheme.obtainStyledAttributes(new int[] {attrResId});

try {
return a.getBoolean(0, resolveBoolRes(defResId));
Expand All @@ -303,8 +289,7 @@ public boolean resolveBoolAttr(@AttrRes int attrResId, @BoolRes int defResId) {
}

public int resolveColorAttr(@AttrRes int attrResId, @ColorRes int defResId) {
mAttrs[0] = attrResId;
TypedArray a = mTheme.obtainStyledAttributes(mAttrs);
TypedArray a = mTheme.obtainStyledAttributes(new int[] {attrResId});

try {
return a.getColor(0, resolveColorRes(defResId));
Expand All @@ -314,8 +299,7 @@ public int resolveColorAttr(@AttrRes int attrResId, @ColorRes int defResId) {
}

public int resolveDimenSizeAttr(@AttrRes int attrResId, @DimenRes int defResId) {
mAttrs[0] = attrResId;
TypedArray a = mTheme.obtainStyledAttributes(mAttrs);
TypedArray a = mTheme.obtainStyledAttributes(new int[] {attrResId});

try {
return a.getDimensionPixelSize(0, resolveDimenSizeRes(defResId));
Expand All @@ -325,8 +309,7 @@ public int resolveDimenSizeAttr(@AttrRes int attrResId, @DimenRes int defResId)
}

public int resolveDimenOffsetAttr(@AttrRes int attrResId, @DimenRes int defResId) {
mAttrs[0] = attrResId;
TypedArray a = mTheme.obtainStyledAttributes(mAttrs);
TypedArray a = mTheme.obtainStyledAttributes(new int[] {attrResId});

try {
return a.getDimensionPixelOffset(0, resolveDimenOffsetRes(defResId));
Expand All @@ -336,8 +319,7 @@ public int resolveDimenOffsetAttr(@AttrRes int attrResId, @DimenRes int defResId
}

public float resolveFloatAttr(@AttrRes int attrResId, @DimenRes int defResId) {
mAttrs[0] = attrResId;
TypedArray a = mTheme.obtainStyledAttributes(mAttrs);
TypedArray a = mTheme.obtainStyledAttributes(new int[] {attrResId});

try {
return a.getDimension(0, resolveFloatRes(defResId));
Expand All @@ -352,8 +334,7 @@ public Drawable resolveDrawableAttr(@AttrRes int attrResId, @DrawableRes int def
return null;
}

mAttrs[0] = attrResId;
TypedArray a = mTheme.obtainStyledAttributes(mAttrs);
TypedArray a = mTheme.obtainStyledAttributes(new int[] {attrResId});

try {
return resolveDrawableRes(a.getResourceId(0, defResId));
Expand All @@ -363,19 +344,12 @@ public Drawable resolveDrawableAttr(@AttrRes int attrResId, @DrawableRes int def
}

final int resolveResIdAttr(@AttrRes int attrResId, int defResId) {
mAttrs[0] = attrResId;
TypedArray a = mTheme.obtainStyledAttributes(mAttrs);
TypedArray a = mTheme.obtainStyledAttributes(new int[] {attrResId});

try {
return a.getResourceId(0, defResId);
} finally {
a.recycle();
}
}

public final void release() {
mResources = null;
mTheme = null;
mResourceCache = null;
}
}
28 changes: 10 additions & 18 deletions litho-sections-core/src/main/java/com/facebook/litho/sections/Section.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,11 @@ protected T loadingEventHandler(EventHandler<LoadingEvent> loadingEventHandler)

public abstract T getThis();

/**
* @return The immutable {@link Section}.
*/
/** @return The immutable {@link Section}. */
public abstract Section build();

protected void release() {
mSection = null;
mResourceResolver.release();
mResourceResolver = null;
}

Expand Down Expand Up @@ -210,17 +207,15 @@ public Section getParent() {
return mParent;
}

/**
* Sets the parent of this {@link Section} in the tree.
*/
/** Sets the parent of this {@link Section} in the tree. */
void setParent(Section parent) {
mParent = parent;
}

/**
* Invalidates The subtree having its root in this {@link Section}. When a subtree is invalidated,
* the {@link OnDiff} will be invoked
* regardless of whether the {@link com.facebook.litho.annotations.Prop}s changed or not.
* the {@link OnDiff} will be invoked regardless of whether the {@link
* com.facebook.litho.annotations.Prop}s changed or not.
*/
void invalidate() {
invalidateInternal(this);
Expand All @@ -243,9 +238,8 @@ void setInvalidated(boolean invalidated) {
}

/**
* @return a clone of this {@link Section}.
* if deepCopy is false the clone won't contain any children or count as it will
* be returned in a pre - ChangeSet generation state.
* @return a clone of this {@link Section}. if deepCopy is false the clone won't contain any
* children or count as it will be returned in a pre - ChangeSet generation state.
*/
public Section makeShallowCopy(boolean deepCopy) {
try {
Expand Down Expand Up @@ -338,11 +332,9 @@ protected StateContainer getStateContainer() {
return null;
}

/**
* Called when this {@link Section} is not in use anymore to release its resources.
*/
/** Called when this {@link Section} is not in use anymore to release its resources. */
void release() {
//TODO release list into a pool t11953296
// TODO release list into a pool t11953296
}

void generateKeyAndSet(SectionContext c, String globalKey) {
Expand Down Expand Up @@ -384,7 +376,7 @@ public String generateUniqueGlobalKeyForChild(Section section, String childKey)

static Map<String, Pair<Section, Integer>> acquireChildrenMap(
@Nullable Section currentComponent) {
//TODO use pools instead t11953296
// TODO use pools instead t11953296
final HashMap<String, Pair<Section, Integer>> childrenMap = new HashMap<>();
if (currentComponent == null) {
return childrenMap;
Expand All @@ -405,7 +397,7 @@ static Map<String, Pair<Section, Integer>> acquireChildrenMap(
}

static void releaseChildrenMap(Map<String, Pair<Section, Integer>> newChildren) {
//TODO use pools t11953296
// TODO use pools t11953296
}

@VisibleForTesting
Expand Down

0 comments on commit c93517c

Please sign in to comment.