From bbb32f3f3c487de5e133b74026df72150974d7a5 Mon Sep 17 00:00:00 2001
From: Taras Tsugrii <ttsugrii@fb.com>
Date: Thu, 2 Aug 2018 03:07:42 -0700
Subject: [Skylark] Improve Skylark interpreter performance.

According to JMH and JIT assembly generated for iterators and index
accesses, iterator methods like `hasNext` and `next` are not optimized
away and result in 3-4X slower execution.

Not using an iterator reduces Buck parse time for FB's internal Android
apps by 7%+.

Closes #5735.

PiperOrigin-RevId: 207073078
---
 .../java/com/google/devtools/build/lib/syntax/EvalUtils.java     | 2 +-
 .../com/google/devtools/build/lib/syntax/FuncallExpression.java  | 9 ++++++---
 2 files changed, 7 insertions(+), 4 deletions(-)

(limited to 'src/main/java/com/google/devtools')

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
index 541413f4c1..faf66bb907 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
@@ -413,7 +413,7 @@ public final class EvalUtils {
               + "--incompatible_string_is_not_iterable=false to temporarily disable this check.");
     }
 
-    ImmutableList.Builder<String> builder = new ImmutableList.Builder<>();
+    ImmutableList.Builder<String> builder = ImmutableList.builderWithExpectedSize(value.length());
     for (char c : value.toCharArray()) {
       builder.add(String.valueOf(c));
     }
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 87991d539c..44e6a67ae9 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
@@ -512,8 +512,9 @@ public final class FuncallExpression extends Expression {
       Map<String, Object> kwargs,
       MethodDescriptor method,
       Environment environment) {
+    ImmutableList<ParamDescriptor> parameters = method.getParameters();
     ImmutableList.Builder<Object> builder =
-        ImmutableList.builderWithExpectedSize(method.getParameters().size() + EXTRA_ARGS_COUNT);
+        ImmutableList.builderWithExpectedSize(parameters.size() + EXTRA_ARGS_COUNT);
     boolean acceptsExtraArgs = method.isAcceptsExtraArgs();
     boolean acceptsExtraKwargs = method.isAcceptsExtraKwargs();
 
@@ -524,9 +525,11 @@ public final class FuncallExpression extends Expression {
     // 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.
-    for (ParamDescriptor param : method.getParameters()) {
+    // Index is used deliberately, since usage of iterators adds a significant overhead
+    for (int i = 0; i < parameters.size(); ++i) {
+      ParamDescriptor param = parameters.get(i);
       SkylarkType type = param.getSkylarkType();
-      Object value = null;
+      Object value;
 
       if (argIndex < args.size() && param.isPositional()) { // Positional args and params remain.
         value = args.get(argIndex);
-- 
cgit v1.2.3