GC reduction: convertStarlarkArgumentsToJavaMethodArguments
- use ArrayList instead of ImmutableList.Builder
This reduces the number of copies by one, because
ImmutableList.Builder.build makes a copy, and toArray makes another
copy, whereas with ArrayList the first copy goes away (the second still
applies).
- use CompactHashSet instead of LinkedHashSet
- avoid creating a new Set if it's going to be empty
note that CompactHashSet actually _increases_ memory consumption for
empty sets, so we need to make sure that we don't do that
On a benchmark, this reduces memory allocation by this method by a
factor of more than 2.
PiperOrigin-RevId: 255959188
diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index 55939a1..6949316 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -488,6 +488,7 @@
":skylarkinterface",
":util",
"//src/main/java/com/google/devtools/build/lib/cmdline",
+ "//src/main/java/com/google/devtools/build/lib/collect/compacthashset",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/profiler",
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
index c0ae74e..6875247 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
@@ -22,8 +22,10 @@
import com.google.common.cache.LoadingCache;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.collect.compacthashset.CompactHashSet;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
import com.google.devtools.build.lib.skylarkinterface.SkylarkInterfaceUtils;
@@ -39,7 +41,6 @@
import java.util.Comparator;
import java.util.HashSet;
import java.util.LinkedHashMap;
-import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@@ -498,15 +499,16 @@
"struct field methods should be handled by DotExpression separately");
ImmutableList<ParamDescriptor> parameters = method.getParameters();
- ImmutableList.Builder<Object> builder =
- ImmutableList.builderWithExpectedSize(parameters.size() + EXTRA_ARGS_COUNT);
+ List<Object> builder = new ArrayList<>(parameters.size() + EXTRA_ARGS_COUNT);
boolean acceptsExtraArgs = method.isAcceptsExtraArgs();
boolean acceptsExtraKwargs = method.isAcceptsExtraKwargs();
int argIndex = 0;
// Process parameters specified in callable.parameters()
- Set<String> keys = new LinkedHashSet<>(kwargs.keySet());
+ // Many methods don't have any kwargs, so don't allocate a new hash set in that case.
+ Set<String> keys =
+ kwargs.isEmpty() ? ImmutableSet.of() : CompactHashSet.create(kwargs.keySet());
// Positional parameters are always enumerated before non-positional parameters,
// And default-valued positional parameters are always enumerated after other positional
// parameters. These invariants are validated by the SkylarkCallable annotation processor.
@@ -541,7 +543,7 @@
}
argIndex++;
} else { // No more positional arguments, or no more positional parameters.
- if (param.isNamed() && keys.remove(param.getName())) {
+ if (param.isNamed() && !keys.isEmpty() && keys.remove(param.getName())) {
// Param specified by keyword argument.
value = kwargs.get(param.getName());
if (!type.contains(value)) {
@@ -607,7 +609,7 @@
}
appendExtraInterpreterArgs(builder, method, this, getLocation(), environment);
- return builder.build().toArray();
+ return builder.toArray();
}
private EvalException unspecifiedParameterException(
@@ -701,9 +703,9 @@
*/
public static List<Object> extraInterpreterArgs(
MethodDescriptor method, @Nullable FuncallExpression ast, Location loc, Environment env) {
- ImmutableList.Builder<Object> builder = ImmutableList.builder();
+ List<Object> builder = new ArrayList<>();
appendExtraInterpreterArgs(builder, method, ast, loc, env);
- return builder.build();
+ return ImmutableList.copyOf(builder);
}
/**
@@ -714,7 +716,7 @@
* @see #extraInterpreterArgs(MethodDescriptor, FuncallExpression, Location, Environment)
*/
private static void appendExtraInterpreterArgs(
- ImmutableList.Builder<Object> builder,
+ List<Object> builder,
MethodDescriptor method,
@Nullable FuncallExpression ast,
Location loc,
