Skip to content

Commit

Permalink
Limit DecodeProducer to at most one decode per 100ms
Browse files Browse the repository at this point in the history
  • Loading branch information
plamenko authored and tyronen committed Apr 1, 2015
1 parent 7798d84 commit 8cf7267
Show file tree
Hide file tree
Showing 6 changed files with 148 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,12 @@

import javax.annotation.concurrent.GuardedBy;

import java.util.Map;

import com.facebook.common.internal.Maps;
import com.facebook.common.internal.VisibleForTesting;
import com.facebook.common.internal.Preconditions;
import com.facebook.common.logging.FLog;

/**
* A shared-reference class somewhat similar to c++ shared_ptr. The underlying value is reference
Expand Down Expand Up @@ -92,7 +96,12 @@
*/
@VisibleForTesting
public class SharedReference<T> {
private static final Class<?> TAG = SharedReference.class;

// Keeps references to all live objects so finalization of those Objects always happens after
// SharedReference first disposes of it. Note, this does not prevent CloseableReference's from
// being finalized when the reference is no longer reachable.
@GuardedBy("itself")
private static final Map<Object, Integer> sLiveObjects = Maps.newIdentityHashMap();

@GuardedBy("this")
private T mValue;
Expand All @@ -112,6 +121,47 @@ public SharedReference(T value, ResourceReleaser<T> resourceReleaser) {
mValue = Preconditions.checkNotNull(value);
mResourceReleaser = Preconditions.checkNotNull(resourceReleaser);
mRefCount = 1;
addLiveReference(value);
}

/**
* Increases the reference count of a live object in the static map. Adds it if it's not
* being held.
*
* @param value the value to add.
*/
private static void addLiveReference(Object value) {
synchronized (sLiveObjects) {
Integer count = sLiveObjects.get(value);
if (count == null) {
sLiveObjects.put(value, 1);
} else {
sLiveObjects.put(value, count + 1);
}
}
}

/**
* Decreases the reference count of live object from the static map. Removes it if it's reference
* count has become 0.
*
* @param value the value to remove.
*/
private static void removeLiveReference(Object value) {
synchronized (sLiveObjects) {
Integer count = sLiveObjects.get(value);
if (count == null) {
// Uh oh.
FLog.wtf(
"SharedReference",
"No entry in sLiveObjects for value of type %s",
value.getClass());
} else if (count == 1) {
sLiveObjects.remove(value);
} else {
sLiveObjects.put(value, count - 1);
}
}
}

/**
Expand Down Expand Up @@ -153,10 +203,13 @@ public synchronized void addReference() {
*/
public void deleteReference() {
if (decreaseRefCount() == 0) {
mResourceReleaser.release(mValue);
T deleted;
synchronized (this) {
deleted = mValue;
mValue = null;
}
mResourceReleaser.release(deleted);
removeLiveReference(deleted);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ public class ImageDecodeOptions {

private static final ImageDecodeOptions DEFAULTS = ImageDecodeOptions.newBuilder().build();

/**
* Decoding of intermediate results for an image won't happen more often that minDecodeIntervalMs.
*/
public final int minDecodeIntervalMs;

/**
* Background color used when converting to image formats that don't support transparency.
*/
Expand All @@ -40,6 +45,7 @@ public class ImageDecodeOptions {
public final boolean useLastFrameForPreview;

ImageDecodeOptions(ImageDecodeOptionsBuilder b) {
this.minDecodeIntervalMs = b.getMinDecodeIntervalMs();
this.backgroundColor = b.getBackgroundColor();
this.forceOldAnimationCode = b.getForceOldAnimationCode();
this.decodePreviewFrame = b.getDecodePreviewFrame();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
*/
public class ImageDecodeOptionsBuilder {

private int mMinDecodeIntervalMs = 100;
private int mBackgroundColor = 0xFFFFFF;
private boolean mForceOldAnimationCode;
private boolean mDecodePreviewFrame;
Expand All @@ -36,6 +37,29 @@ public ImageDecodeOptionsBuilder setFrom(ImageDecodeOptions options) {
return this;
}

/**
* Sets the minimum decode interval.
*
* <p/> Decoding of intermediate results won't happen more often that intervalMs. If another
* intermediate result comes too soon, it will be decoded only after intervalMs since the last
* decode. If there were more intermediate results in between, only the last one gets decoded.
* @param intervalMs the minimum decode interval in milliseconds
* @return this builder
*/
public ImageDecodeOptionsBuilder setMinDecodeIntervalMs(int intervalMs) {
mMinDecodeIntervalMs = intervalMs;
return this;
}

/**
* Gets the minimum decode interval.
*
* @return the minimum decode interval in milliseconds
*/
public int getMinDecodeIntervalMs() {
return mMinDecodeIntervalMs;
}

/**
* Sets the background color used when converting to image formats that don't support
* transparency.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,13 @@

package com.facebook.imagepipeline.core;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

import android.app.ActivityManager;
import android.content.Context;
import android.graphics.Rect;
import android.os.Build;

import com.facebook.cache.disk.DiskCacheConfig;
import com.facebook.common.internal.Preconditions;
Expand Down Expand Up @@ -137,7 +135,7 @@ public Boolean get() {
builder.mPoolFactory;
mProgressiveJpegConfig =
builder.mProgressiveJpegConfig == null ?
getDefaultProgressiveJpegConfig() :
new SimpleProgressiveJpegConfig() :
builder.mProgressiveJpegConfig;
mRequestListeners =
builder.mRequestListeners == null ?
Expand Down Expand Up @@ -195,12 +193,6 @@ private static DiskCacheConfig getDefaultMainDiskCacheConfig(Context context) {
.build();
}

private static ProgressiveJpegConfig getDefaultProgressiveJpegConfig() {
// By default, don't return images progressively at all.
return new SimpleProgressiveJpegConfig(
Collections.unmodifiableList(new ArrayList<Integer>()), 0);
}

public Supplier<MemoryCacheParams> getBitmapMemoryCacheParamsSupplier() {
return mBitmapMemoryCacheParamsSupplier;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,25 @@

package com.facebook.imagepipeline.decoder;

import java.util.ArrayList;
import java.util.List;

import com.facebook.imagepipeline.decoder.ProgressiveJpegConfig;
import com.facebook.imagepipeline.image.ImmutableQualityInfo;
import com.facebook.imagepipeline.image.QualityInfo;

/**
* Simple {@link ProgressiveJpegConfig} with predefined scans to decode and good-enough scan number.
*
* <p/> If no specific scans to decode are provided, every scan is allowed to be decoded.
*/
public class SimpleProgressiveJpegConfig implements ProgressiveJpegConfig {
private final List<Integer> mScansToDecode;
private final int mGoodEnoughScanNumber;

public SimpleProgressiveJpegConfig() {
this(new ArrayList<Integer>(), 0);
}

public SimpleProgressiveJpegConfig(
List<Integer> scansToDecode,
int goodEnoughScanNumber) {
Expand All @@ -31,6 +37,9 @@ public SimpleProgressiveJpegConfig(

@Override
public int getNextScanNumberToDecode(int scanNumber) {
if (mScansToDecode == null || mScansToDecode.isEmpty()) {
return scanNumber + 1;
}
for (int i = 0; i < mScansToDecode.size(); i++) {
if (mScansToDecode.get(i) > scanNumber) {
return mScansToDecode.get(i);
Expand Down
Loading

0 comments on commit 8cf7267

Please sign in to comment.