Skip to content

Commit

Permalink
A couple tweaks to ModelLoaderRegistry to improve potential contention.
Browse files Browse the repository at this point in the history
1) Synchronize only the inner method that accesses the cache, as the outer call
that loops and filters the loaders appears to not have threading issues.
2) Probably less of an impact since I imagine the loops are generally small,
but only allocate an ArrayList once you find a matching loader, and allocate it
with size - i which is the max possible remaining in the loop, rather than
always allocating it to size.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=205877123
  • Loading branch information
jnlopar authored and sjudd committed Jul 30, 2018
1 parent 229cb11 commit 62e6c11
Showing 1 changed file with 9 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,19 @@ private <Model, Data> void tearDown(
}

@NonNull
public synchronized <A> List<ModelLoader<A, ?>> getModelLoaders(@NonNull A model) {
public <A> List<ModelLoader<A, ?>> getModelLoaders(@NonNull A model) {
List<ModelLoader<A, ?>> modelLoaders = getModelLoadersForClass(getClass(model));
int size = modelLoaders.size();
List<ModelLoader<A, ?>> filteredLoaders = new ArrayList<>(size);
boolean isEmpty = true;
List<ModelLoader<A, ?>> filteredLoaders = Collections.emptyList();
//noinspection ForLoopReplaceableByForEach to improve perf
for (int i = 0; i < size; i++) {
ModelLoader<A, ?> loader = modelLoaders.get(i);
if (loader.handles(model)) {
if (isEmpty) {
filteredLoaders = new ArrayList<>(size - i);
isEmpty = false;
}
filteredLoaders.add(loader);
}
}
Expand All @@ -92,7 +97,8 @@ public synchronized List<Class<?>> getDataClasses(@NonNull Class<?> modelClass)
}

@NonNull
private <A> List<ModelLoader<A, ?>> getModelLoadersForClass(@NonNull Class<A> modelClass) {
private synchronized <A> List<ModelLoader<A, ?>> getModelLoadersForClass(
@NonNull Class<A> modelClass) {
List<ModelLoader<A, ?>> loaders = cache.get(modelClass);
if (loaders == null) {
loaders = Collections.unmodifiableList(multiModelLoaderFactory.build(modelClass));
Expand Down

0 comments on commit 62e6c11

Please sign in to comment.