WidgetPreviewLoader concurrency issue / Caching improvement
1) Concurrency issue: unused bitmap was not properly synchronized
which caused concurrency issue. Hence, leading current widget tray
implementation to not use it. (a.k.a. cancel(false)).
Issue fixed and now using the unused bitmap pool.
2) Caching improvement: LoadedBitmap cache was a legacy support system
for the old widget tray implementation. On our latest implementation,
cache and recycled view is completely being managed by the recycler view.
Hence removed.
Change-Id: I843e6a286b676f283172f2c1ef5cbeed0a9fb17f
diff --git a/src/com/android/launcher3/WidgetPreviewLoader.java b/src/com/android/launcher3/WidgetPreviewLoader.java
index 5c3ed92..412cbcd 100644
--- a/src/com/android/launcher3/WidgetPreviewLoader.java
+++ b/src/com/android/launcher3/WidgetPreviewLoader.java
@@ -34,7 +34,6 @@
import com.android.launcher3.util.Thunk;
import com.android.launcher3.widget.WidgetCell;
-import java.lang.ref.WeakReference;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
@@ -51,8 +50,13 @@
private static final float WIDGET_PREVIEW_ICON_PADDING_PERCENTAGE = 0.25f;
private final HashMap<String, long[]> mPackageVersions = new HashMap<>();
- private final HashMap<WidgetCacheKey, WeakReference<Bitmap>> mLoadedPreviews = new HashMap<>();
- private Set<Bitmap> mUnusedBitmaps = Collections.newSetFromMap(new WeakHashMap<Bitmap, Boolean>());
+
+ /**
+ * Weak reference objects, do not prevent their referents from being made finalizable,
+ * finalized, and then reclaimed.
+ */
+ private Set<Bitmap> mUnusedBitmaps =
+ Collections.newSetFromMap(new WeakHashMap<Bitmap, Boolean>());
private final Context mContext;
private final IconCache mIconCache;
@@ -84,18 +88,9 @@
String size = previewWidth + "x" + previewHeight;
WidgetCacheKey key = getObjectKey(o, size);
- // Check if we have the preview loaded or not.
- synchronized (mLoadedPreviews) {
- WeakReference<Bitmap> ref = mLoadedPreviews.get(key);
- if (ref != null && ref.get() != null) {
- immediateResult[0] = ref.get();
- return new PreviewLoadRequest(null, key);
- }
- }
-
PreviewLoadTask task = new PreviewLoadTask(key, o, previewWidth, previewHeight, caller);
task.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
- return new PreviewLoadRequest(task, key);
+ return new PreviewLoadRequest(task);
}
/**
@@ -192,22 +187,6 @@
mPackageVersions.remove(packageName);
}
- synchronized (mLoadedPreviews) {
- Set<WidgetCacheKey> keysToRemove = new HashSet<>();
- for (WidgetCacheKey key : mLoadedPreviews.keySet()) {
- if (key.componentName.getPackageName().equals(packageName) && key.user.equals(user)) {
- keysToRemove.add(key);
- }
- }
-
- for (WidgetCacheKey key : keysToRemove) {
- WeakReference<Bitmap> req = mLoadedPreviews.remove(key);
- if (req != null && req.get() != null) {
- mUnusedBitmaps.add(req.get());
- }
- }
- }
-
try {
mDb.getWritableDatabase().delete(CacheDb.TABLE_NAME,
CacheDb.COLUMN_PACKAGE + " = ? AND " + CacheDb.COLUMN_USER + " = ?",
@@ -549,26 +528,25 @@
public class PreviewLoadRequest {
private final PreviewLoadTask mTask;
- private final WidgetCacheKey mKey;
- public PreviewLoadRequest(PreviewLoadTask task, WidgetCacheKey key) {
+ public PreviewLoadRequest(PreviewLoadTask task) {
mTask = task;
- mKey = key;
}
- public void cancel(boolean recycleImage) {
+ public void cleanup() {
if (mTask != null) {
mTask.cancel(true);
}
- if (recycleImage) {
- synchronized(mLoadedPreviews) {
- WeakReference<Bitmap> result = mLoadedPreviews.remove(mKey);
- if (result != null && result.get() != null) {
- mUnusedBitmaps.add(result.get());
- }
- }
+ if (mTask.mBitmap == null) {
+ return;
}
+
+ // The preview is no longer bound to any view, move it to {@link WeakReference} list.
+ synchronized(mUnusedBitmaps) {
+ mUnusedBitmaps.add(mTask.mBitmap);
+ }
+ mTask.mBitmap = null;
}
}
@@ -579,6 +557,7 @@
private final int mPreviewHeight;
private final int mPreviewWidth;
private final WidgetCell mCaller;
+ private Bitmap mBitmap;
PreviewLoadTask(WidgetCacheKey key, Object info, int previewWidth,
int previewHeight, WidgetCell caller) {
@@ -597,7 +576,6 @@
protected Bitmap doInBackground(Void... params) {
Bitmap unusedBitmap = null;
- // TODO(hyunyoungs): Figure out why this path causes concurrency issue.
synchronized (mUnusedBitmaps) {
// Check if we can use a bitmap
for (Bitmap candidate : mUnusedBitmaps) {
@@ -638,10 +616,7 @@
@Override
protected void onPostExecute(Bitmap result) {
- synchronized(mLoadedPreviews) {
- mLoadedPreviews.put(mKey, new WeakReference<Bitmap>(result));
- }
-
+ mBitmap = result;
mCaller.applyPreview(result);
}
}
diff --git a/src/com/android/launcher3/widget/WidgetCell.java b/src/com/android/launcher3/widget/WidgetCell.java
index 7c7d982..2df170e 100644
--- a/src/com/android/launcher3/widget/WidgetCell.java
+++ b/src/com/android/launcher3/widget/WidgetCell.java
@@ -94,18 +94,25 @@
mOriginalImagePadding.right = mWidgetImage.getPaddingRight();
mOriginalImagePadding.bottom = mWidgetImage.getPaddingBottom();
- // Ensure we are using the right text size
- DeviceProfile profile = LauncherAppState.getInstance().getDynamicGrid().getDeviceProfile();
mWidgetName = ((TextView) findViewById(R.id.widget_name));
mWidgetDims = ((TextView) findViewById(R.id.widget_dims));
}
- public void reset() {
+ /**
+ * Called to clear the view and free attached resources. (e.g., {@link Bitmap}
+ */
+ public void clear() {
+ if (DEBUG) {
+ Log.d(TAG, "reset called on:" + mWidgetName.getText());
+ }
mWidgetImage.setImageDrawable(null);
mWidgetName.setText(null);
mWidgetDims.setText(null);
- cancelLoader(false);
+ if (mActiveRequest != null) {
+ mActiveRequest.cleanup();
+ mActiveRequest = null;
+ }
}
/**
@@ -211,13 +218,6 @@
return Math.min(size[0], info.spanX * cellWidth);
}
- private void cancelLoader(boolean recycleImage) {
- if (mActiveRequest != null) {
- mActiveRequest.cancel(recycleImage);
- mActiveRequest = null;
- }
- }
-
/**
* Helper method to get the string info of the tag.
*/
diff --git a/src/com/android/launcher3/widget/WidgetsContainerView.java b/src/com/android/launcher3/widget/WidgetsContainerView.java
index 27a3ea1..8090c88 100644
--- a/src/com/android/launcher3/widget/WidgetsContainerView.java
+++ b/src/com/android/launcher3/widget/WidgetsContainerView.java
@@ -26,7 +26,6 @@
import android.support.v7.widget.RecyclerView;
import android.util.AttributeSet;
import android.util.Log;
-import android.view.MotionEvent;
import android.view.View;
import android.widget.FrameLayout;
import android.widget.ImageView;
@@ -61,9 +60,6 @@
private static final String TAG = "WidgetsContainerView";
private static final boolean DEBUG = false;
- /* {@link RecyclerView} will keep following # of views in cache, before recycling. */
- private static final int WIDGET_CACHE_SIZE = 2;
-
private static final int SPRING_MODE_DELAY_MS = 150;
/* Global instances that are used inside this container. */
@@ -119,7 +115,6 @@
mView = (RecyclerView) findViewById(R.id.widgets_list_view);
mView.setAdapter(mAdapter);
mView.setLayoutManager(new LinearLayoutManager(getContext()));
- mView.setItemViewCacheSize(WIDGET_CACHE_SIZE);
mPadding.set(getPaddingLeft(), getPaddingTop(), getPaddingRight(),
getPaddingBottom());
diff --git a/src/com/android/launcher3/widget/WidgetsListAdapter.java b/src/com/android/launcher3/widget/WidgetsListAdapter.java
index a5b2aff..051a3ab 100644
--- a/src/com/android/launcher3/widget/WidgetsListAdapter.java
+++ b/src/com/android/launcher3/widget/WidgetsListAdapter.java
@@ -120,7 +120,9 @@
mIconCache.getTitleAndIconForApp(infoOut.packageName,
UserHandleCompat.myUserHandle(), false /* useLowResIcon */, infoOut);
}
- ((TextView) holder.getContent().findViewById(R.id.section)).setText(infoOut.title);
+
+ TextView tv = ((TextView) holder.getContent().findViewById(R.id.section));
+ tv.setText(infoOut.title);
ImageView iv = (ImageView) holder.getContent().findViewById(R.id.section_image);
iv.setImageBitmap(infoOut.iconBitmap);
@@ -149,7 +151,7 @@
@Override
public WidgetsRowViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
if (DEBUG) {
- Log.v(TAG, String.format("\nonCreateViewHolder, [widget#=%d]", viewType));
+ Log.v(TAG, "\nonCreateViewHolder");
}
ViewGroup container = (ViewGroup) mLayoutInflater.inflate(
@@ -159,11 +161,15 @@
@Override
public void onViewRecycled(WidgetsRowViewHolder holder) {
+ if (DEBUG) {
+ Log.v(TAG, String.format("onViewDetachedFromWindow, [pos=%d]",
+ holder.getAdapterPosition()));
+ }
ViewGroup row = ((ViewGroup) holder.getContent().findViewById(R.id.widgets_cell_list));
for (int i = 0; i < row.getChildCount(); i++) {
WidgetCell widget = (WidgetCell) row.getChildAt(i);
- widget.reset();
+ widget.clear();
}
}
diff --git a/src/com/android/launcher3/widget/WidgetsRowViewHolder.java b/src/com/android/launcher3/widget/WidgetsRowViewHolder.java
index 99a192c..249559a 100644
--- a/src/com/android/launcher3/widget/WidgetsRowViewHolder.java
+++ b/src/com/android/launcher3/widget/WidgetsRowViewHolder.java
@@ -25,7 +25,7 @@
ViewGroup mContent;
- public WidgetsRowViewHolder(ViewGroup v) {
+ public WidgetsRowViewHolder(ViewGroup v) {
super(v);
mContent = v;
}