From 60c288a5fb06f13d47de066c1e252339bd181e20 Mon Sep 17 00:00:00 2001
From: Taras Tsugrii <ttsugrii@fb.com>
Date: Mon, 30 Jul 2018 13:25:57 -0700
Subject: [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
---
 .../devtools/build/lib/syntax/FuncallExpression.java | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

(limited to 'src/main/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 814b329a80..48c52f5791 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 @@ public final class FuncallExpression extends Expression {
                 }
               });
 
+  // *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 @@ public final class FuncallExpression extends Expression {
       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 @@ public final class FuncallExpression extends Expression {
       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 @@ public final class FuncallExpression extends Expression {
                 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 @@ public final class FuncallExpression extends Expression {
 
     // 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));
 
-- 
cgit v1.2.3