commit 60c288a5fb06f13d47de066c1e252339bd181e20
author Taras Tsugrii <ttsugrii@fb.com> Mon Jul 30 13:25:57 2018 -0700
committer Copybara-Service <copybara-piper@google.com> Mon Jul 30 13:27:26 2018 -0700
tree 2b3954ae1ce2caa9458f32ab162abff2a97f923c
parent df8bf902e642ad8e8244e7fd58f1c36a6dfadb6c

[Skylark] Size argument list builder to avoid allocations.

This change is focused on 2 things:
- avoid creating builders in case they don't end up being used
- create builders using the maximum expected size to avoid intermediate
  allocations to accommodate more elements

Closes #5694.

PiperOrigin-RevId: 206636046

src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java

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 814b329..48c52f5 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
@@ -147,6 +147,9 @@
                 }
               });
 
+  // *args, **kwargs, location, ast, environment, skylark semantics
+  private static final int EXTRA_ARGS_COUNT = 6;
+
   /**
    * Returns a map of methods and corresponding SkylarkCallable annotations of the methods of the
    * classObj class reachable from Skylark.
@@ -554,9 +557,8 @@
       Map<String, Object> kwargs,
       MethodDescriptor method,
       Environment environment) {
-    ImmutableList.Builder<Object> builder = ImmutableList.builder();
-    ImmutableList.Builder<Object> extraArgsBuilder = ImmutableList.builder();
-    ImmutableMap.Builder<String, Object> extraKwargsBuilder = ImmutableMap.builder();
+    ImmutableList.Builder<Object> builder =
+        ImmutableList.builderWithExpectedSize(method.getParameters().size() + EXTRA_ARGS_COUNT);
     boolean acceptsExtraArgs = method.isAcceptsExtraArgs();
     boolean acceptsExtraKwargs = method.isAcceptsExtraKwargs();
 
@@ -611,11 +613,15 @@
       builder.add(value);
     }
 
+    ImmutableList<Object> extraArgs = ImmutableList.of();
     if (argIndex < args.size()) {
       if (acceptsExtraArgs) {
+        ImmutableList.Builder<Object> extraArgsBuilder =
+            ImmutableList.builderWithExpectedSize(args.size() - argIndex);
         for (; argIndex < args.size(); argIndex++) {
           extraArgsBuilder.add(args.get(argIndex));
         }
+        extraArgs = extraArgsBuilder.build();
       } else {
         return ArgumentListConversionResult.fromError(
             String.format(
@@ -623,11 +629,15 @@
                 argIndex, args.size()));
       }
     }
+    ImmutableMap<String, Object> extraKwargs = ImmutableMap.of();
     if (!keys.isEmpty()) {
       if (acceptsExtraKwargs) {
+        ImmutableMap.Builder<String, Object> extraKwargsBuilder =
+            ImmutableMap.builderWithExpectedSize(keys.size());
         for (String key : keys) {
           extraKwargsBuilder.put(key, kwargs.get(key));
         }
+        extraKwargs = extraKwargsBuilder.build();
       } else {
         return ArgumentListConversionResult.fromError(
             String.format(
@@ -639,10 +649,10 @@
 
     // Then add any skylark-interpreter arguments (for example kwargs or the Environment).
     if (acceptsExtraArgs) {
-      builder.add(Tuple.copyOf(extraArgsBuilder.build()));
+      builder.add(Tuple.copyOf(extraArgs));
     }
     if (acceptsExtraKwargs) {
-      builder.add(SkylarkDict.copyOf(environment, extraKwargsBuilder.build()));
+      builder.add(SkylarkDict.copyOf(environment, extraKwargs));
     }
     builder.addAll(extraInterpreterArgs(method, this, getLocation(), environment));