Skip to content

Commit d066c11

Browse files
dnkoutsodnkoutso
committed
Merge pull request square#398 from square/dimitris/stop
Stop allocating array lists for each hunter.
2 parents c2bfcbb + 0d66dea commit d066c11

File tree

5 files changed

+136
-29
lines changed

5 files changed

+136
-29
lines changed

‎.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ lib
1515
target
1616
pom.xml.*
1717
release.properties
18+
coverage.ec
1819

1920
# IntelliJ
2021
.idea

‎picasso/src/main/java/com/squareup/picasso/BitmapHunter.java

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,10 @@ abstract class BitmapHunter implements Runnable {
5757
final Stats stats;
5858
final String key;
5959
final Request data;
60-
final List<Action> actions;
6160
final boolean skipMemoryCache;
6261

62+
Action action;
63+
List<Action> actions;
6364
Bitmap result;
6465
Future<?> future;
6566
Picasso.LoadedFrom loadedFrom;
@@ -74,8 +75,7 @@ abstract class BitmapHunter implements Runnable {
7475
this.key = action.getKey();
7576
this.data = action.getData();
7677
this.skipMemoryCache = action.skipCache;
77-
this.actions = new ArrayList<Action>(4);
78-
attach(action);
78+
this.action = action;
7979
}
8080

8181
protected void setExifRotation(int exifRotation) {
@@ -149,15 +149,29 @@ Bitmap hunt() throws IOException {
149149
}
150150

151151
void attach(Action action) {
152+
if (this.action == null) {
153+
this.action = action;
154+
return;
155+
}
156+
if (actions == null) {
157+
actions = new ArrayList<Action>(3);
158+
}
152159
actions.add(action);
153160
}
154161

155162
void detach(Action action) {
156-
actions.remove(action);
163+
if (this.action == action) {
164+
this.action = null;
165+
} else if (actions != null) {
166+
actions.remove(action);
167+
}
157168
}
158169

159170
boolean cancel() {
160-
return actions.isEmpty() && future != null && future.cancel(false);
171+
return action == null
172+
&& (actions == null || actions.isEmpty())
173+
&& future != null
174+
&& future.cancel(false);
161175
}
162176

163177
boolean isCancelled() {
@@ -184,6 +198,10 @@ Request getData() {
184198
return data;
185199
}
186200

201+
Action getAction() {
202+
return action;
203+
}
204+
187205
List<Action> getActions() {
188206
return actions;
189207
}

‎picasso/src/main/java/com/squareup/picasso/Picasso.java

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -294,8 +294,13 @@ Bitmap quickMemoryCacheCheck(String key) {
294294
}
295295

296296
void complete(BitmapHunter hunter) {
297+
Action single = hunter.getAction();
297298
List<Action> joined = hunter.getActions();
298-
if (joined.isEmpty()) {
299+
300+
boolean hasMultiple = joined != null && !joined.isEmpty();
301+
boolean shouldDeliver = single != null || hasMultiple;
302+
303+
if (!shouldDeliver) {
299304
return;
300305
}
301306

@@ -304,20 +309,15 @@ void complete(BitmapHunter hunter) {
304309
Bitmap result = hunter.getResult();
305310
LoadedFrom from = hunter.getLoadedFrom();
306311

307-
//noinspection ForLoopReplaceableByForEach
308-
for (int i = 0, n = joined.size(); i < n; i++) {
309-
Action join = joined.get(i);
310-
if (join.isCancelled()) {
311-
continue;
312-
}
313-
targetToAction.remove(join.getTarget());
314-
if (result != null) {
315-
if (from == null) {
316-
throw new AssertionError("LoadedFrom cannot be null.");
317-
}
318-
join.complete(result, from);
319-
} else {
320-
join.error();
312+
if (single != null) {
313+
deliverAction(result, from, single);
314+
}
315+
316+
if (hasMultiple) {
317+
//noinspection ForLoopReplaceableByForEach
318+
for (int i = 0, n = joined.size(); i < n; i++) {
319+
Action join = joined.get(i);
320+
deliverAction(result, from, join);
321321
}
322322
}
323323

@@ -326,6 +326,21 @@ void complete(BitmapHunter hunter) {
326326
}
327327
}
328328

329+
private void deliverAction(Bitmap result, LoadedFrom from, Action action) {
330+
if (action.isCancelled()) {
331+
return;
332+
}
333+
targetToAction.remove(action.getTarget());
334+
if (result != null) {
335+
if (from == null) {
336+
throw new AssertionError("LoadedFrom cannot be null.");
337+
}
338+
action.complete(result, from);
339+
} else {
340+
action.error();
341+
}
342+
}
343+
329344
private void cancelExistingRequest(Object target) {
330345
Action action = targetToAction.remove(target);
331346
if (action != null) {

‎picasso/src/test/java/com/squareup/picasso/BitmapHunterTest.java

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -163,33 +163,68 @@ public class BitmapHunterTest {
163163
assertThat(result).isEqualTo(BITMAP_1);
164164
}
165165

166-
@Test public void attachRequest() throws Exception {
166+
@Test public void attachSingleRequest() throws Exception {
167+
Action action1 = mockAction(URI_KEY_1, URI_1, mockImageViewTarget());
168+
BitmapHunter hunter = new TestableBitmapHunter(picasso, dispatcher, cache, stats, action1);
169+
assertThat(hunter.action).isEqualTo(action1);
170+
hunter.detach(action1);
171+
hunter.attach(action1);
172+
assertThat(hunter.action).isEqualTo(action1);
173+
assertThat(hunter.actions).isNull();
174+
}
175+
176+
@Test public void attachMultipleRequests() throws Exception {
167177
Action action1 = mockAction(URI_KEY_1, URI_1, mockImageViewTarget());
168178
Action action2 = mockAction(URI_KEY_1, URI_1, mockImageViewTarget());
169179
BitmapHunter hunter = new TestableBitmapHunter(picasso, dispatcher, cache, stats, action1);
170-
assertThat(hunter.actions).hasSize(1);
180+
assertThat(hunter.actions).isNull();
171181
hunter.attach(action2);
172-
assertThat(hunter.actions).hasSize(2);
182+
assertThat(hunter.actions).isNotNull().hasSize(1);
173183
}
174184

175-
@Test public void detachRequest() throws Exception {
185+
@Test public void detachSingleRequest() throws Exception {
176186
Action action = mockAction(URI_KEY_1, URI_1, mockImageViewTarget());
177187
BitmapHunter hunter = new TestableBitmapHunter(picasso, dispatcher, cache, stats, action);
178-
assertThat(hunter.actions).hasSize(1);
188+
assertThat(hunter.action).isNotNull();
179189
hunter.detach(action);
180-
assertThat(hunter.actions).isEmpty();
190+
assertThat(hunter.action).isNull();
191+
}
192+
193+
@Test public void detachMutlipleRequests() throws Exception {
194+
Action action = mockAction(URI_KEY_1, URI_1, mockImageViewTarget());
195+
Action action2 = mockAction(URI_KEY_1, URI_1, mockImageViewTarget());
196+
BitmapHunter hunter = new TestableBitmapHunter(picasso, dispatcher, cache, stats, action);
197+
hunter.attach(action2);
198+
hunter.detach(action2);
199+
assertThat(hunter.action).isNotNull();
200+
assertThat(hunter.actions).isNotNull().isEmpty();
201+
hunter.detach(action);
202+
assertThat(hunter.action).isNull();
203+
}
204+
205+
@Test public void cancelSingleRequest() throws Exception {
206+
Action action1 = mockAction(URI_KEY_1, URI_1, mockImageViewTarget());
207+
BitmapHunter hunter = new TestableBitmapHunter(picasso, dispatcher, cache, stats, action1);
208+
hunter.future = new FutureTask<Object>(mock(Runnable.class), mock(Object.class));
209+
assertThat(hunter.isCancelled()).isFalse();
210+
assertThat(hunter.cancel()).isFalse();
211+
hunter.detach(action1);
212+
assertThat(hunter.cancel()).isTrue();
213+
assertThat(hunter.isCancelled()).isTrue();
181214
}
182215

183-
@Test public void cancelRequest() throws Exception {
216+
@Test public void cancelMultipleRequests() throws Exception {
184217
Action action1 = mockAction(URI_KEY_1, URI_1, mockImageViewTarget());
185218
Action action2 = mockAction(URI_KEY_1, URI_1, mockImageViewTarget());
186219
BitmapHunter hunter = new TestableBitmapHunter(picasso, dispatcher, cache, stats, action1);
187220
hunter.future = new FutureTask<Object>(mock(Runnable.class), mock(Object.class));
188221
hunter.attach(action2);
222+
assertThat(hunter.isCancelled()).isFalse();
189223
assertThat(hunter.cancel()).isFalse();
190224
hunter.detach(action1);
191225
hunter.detach(action2);
192226
assertThat(hunter.cancel()).isTrue();
227+
assertThat(hunter.isCancelled()).isTrue();
193228
}
194229

195230
// ---------------------------------------

‎picasso/src/test/java/com/squareup/picasso/PicassoTest.java

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@
5252
import static org.mockito.Mockito.when;
5353
import static org.mockito.MockitoAnnotations.initMocks;
5454

55-
@RunWith(RobolectricTestRunner.class) @Config(manifest = Config.NONE)
55+
@RunWith(RobolectricTestRunner.class)
56+
@Config(manifest = Config.NONE)
5657
public class PicassoTest {
5758

5859
@Mock Context context;
@@ -122,14 +123,51 @@ public class PicassoTest {
122123
verify(listener).onImageLoadFailed(picasso, URI_1, exception);
123124
}
124125

125-
@Test public void completeSkipsIfNoActions() throws Exception {
126+
@Test public void completeDeliversToSingle() throws Exception {
127+
Action action = mockAction(URI_KEY_1, URI_1, mockImageViewTarget());
126128
BitmapHunter hunter = mockHunter(URI_KEY_1, BITMAP_1, false);
129+
when(hunter.getLoadedFrom()).thenReturn(MEMORY);
130+
when(hunter.getAction()).thenReturn(action);
127131
when(hunter.getActions()).thenReturn(Collections.<Action>emptyList());
128132
picasso.complete(hunter);
133+
verify(action).complete(BITMAP_1, MEMORY);
134+
}
135+
136+
@Test public void completeDeliversToSingleAndMultiple() throws Exception {
137+
Action action = mockAction(URI_KEY_1, URI_1, mockImageViewTarget());
138+
Action action2 = mockAction(URI_KEY_1, URI_1, mockImageViewTarget());
139+
BitmapHunter hunter = mockHunter(URI_KEY_1, BITMAP_1, false);
140+
when(hunter.getLoadedFrom()).thenReturn(MEMORY);
141+
when(hunter.getAction()).thenReturn(action);
142+
when(hunter.getActions()).thenReturn(Arrays.asList(action2));
143+
picasso.complete(hunter);
144+
verify(action).complete(BITMAP_1, MEMORY);
145+
verify(action2).complete(BITMAP_1, MEMORY);
146+
}
147+
148+
@Test public void completeSkipsIfNoActions() throws Exception {
149+
BitmapHunter hunter = mockHunter(URI_KEY_1, BITMAP_1, false);
150+
picasso.complete(hunter);
151+
verify(hunter).getAction();
129152
verify(hunter).getActions();
130153
verifyNoMoreInteractions(hunter);
131154
}
132155

156+
@Test public void loadedFromIsNullThrows() throws Exception {
157+
Action action = mockAction(URI_KEY_1, URI_1, mockImageViewTarget());
158+
BitmapHunter hunter = mockHunter(URI_KEY_1, BITMAP_1, false);
159+
when(hunter.getAction()).thenReturn(action);
160+
boolean caught = false;
161+
try {
162+
picasso.complete(hunter);
163+
} catch (AssertionError error) {
164+
caught = true;
165+
}
166+
if (!caught) {
167+
fail("Calling complete() with null LoadedFrom should throw");
168+
}
169+
}
170+
133171
@Test public void cancelExistingRequestWithUnknownTarget() throws Exception {
134172
ImageView target = mockImageViewTarget();
135173
Action action = mockAction(URI_KEY_1, URI_1, target);

0 commit comments

Comments
 (0)