Skip to content

Commit

Permalink
Change RealMultibinder#doProvision to use ImmutableSet#buildWithExpec…
Browse files Browse the repository at this point in the history
…tedSize, and avoid an extra T[] array allocation when permitDuplicates==true.

Since the normal usage should be that the items in the values array are all unique, and ImmutableSet#copyOf will create an initial list of max(4, sqrt(length)) if length>4, but then grows that list to length (or larger) if the array was all unique (and then resize is back down during build()).

While here, the T[] values array is also only needed to be filled in when permitDuplicates==false (for the error message if duplicates were detected).

PiperOrigin-RevId: 592670124
  • Loading branch information
java-team-github-bot authored and Guice Team committed Jan 10, 2024
1 parent e23d3b4 commit 73c8309
Showing 1 changed file with 19 additions and 7 deletions.
26 changes: 19 additions & 7 deletions core/src/com/google/inject/internal/RealMultibinder.java
Original file line number Diff line number Diff line change
Expand Up @@ -236,21 +236,33 @@ protected ImmutableSet<T> doProvision(InternalContext context, Dependency<?> dep
// if localInjectors == null, then we have no bindings so return the empty set.
return ImmutableSet.of();
}
// Ideally we would just add to an ImmutableSet.Builder, but if we did that and there were
// duplicates we wouldn't be able to tell which one was the duplicate. So to manage this we
// first put everything into an array and then construct the set. This way if something gets
// dropped we can figure out what it is.

// If duplicates aren't permitted, we need to capture the original values in order to show a
// meaningful error message to users (if duplicates were encountered).
@SuppressWarnings("unchecked")
T[] values = (T[]) new Object[localInjectors.length];
T[] values = !permitDuplicates ? (T[]) new Object[localInjectors.length] : null;

// Avoid ImmutableSet.copyOf(T[]), because it assumes there'll be duplicates in the input, but
// in the usual case of permitDuplicates==false, we know the exact size must be
// `localInjector.length` (otherwise we fail). This uses `builderWithExpectedSize` to avoid
// the overhead of copyOf or an unknown builder size. If permitDuplicates==true, this will
// assume a potentially larger size (but never a smaller size), and `build` will then reduce
// as necessary.
ImmutableSet.Builder<T> setBuilder =
ImmutableSet.<T>builderWithExpectedSize(localInjectors.length);
for (int i = 0; i < localInjectors.length; i++) {
SingleParameterInjector<T> parameterInjector = localInjectors[i];
T newValue = parameterInjector.inject(context);
if (newValue == null) {
throw newNullEntryException(i);
}
values[i] = newValue;
if (!permitDuplicates) {
values[i] = newValue;
}
setBuilder.add(newValue);
}
ImmutableSet<T> set = ImmutableSet.copyOf(values);
ImmutableSet<T> set = setBuilder.build();

// There are fewer items in the set than the array. Figure out which one got dropped.
if (!permitDuplicates && set.size() < values.length) {
throw newDuplicateValuesException(values);
Expand Down

0 comments on commit 73c8309

Please sign in to comment.