Optimize GC churn of parameter bindings performed before each user de…

…fined function call.

This is the 2nd attempt at this commit. The first attempt (f101348) was rolled back because it introduced the following two bugs:
(1) The side effects of Environment#enterScope are relevant: it creates and stores a new Continuation that has a reference to the set currently referenced by 'knownGlobalVariables', and then overwrites the value of the variable. When there are e.g. nested function calls, 'knownGlobalVariables' will be wrong in the Environment used to stage the inner call (see the added test for an example).
(2) The finally block in UserDefinedFunction#call assumes the env.enterScope was called. Because of the EvalException (incorrectly) thrown due to (1), this is no longer true.

I restructured the code such that (2) isn't possible and I also added a unit test that would have caught the two bugs.

In my first attempt, I was doing too much - I was also trying to save the CPU-costs in the env.update call (dispatches to the just-created lexical frame, and calls LexicalFrame#put, which does an unnecessary mutability sanity check, etc) and in doing so completely missed the above bugs. Sorry.

RELNOTES: None
PiperOrigin-RevId: 188411737

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

@@ -142,6 +142,11 @@ static LexicalFrame create(Mutability mutability) {
           ? ImmutableEmptyLexicalFrame.INSTANCE
           : new MutableLexicalFrame(mutability);
     }
+
+    static LexicalFrame createForUserDefinedFunctionCall(Mutability mutability, int numArgs) {
+      Preconditions.checkState(!mutability.isFrozen());
+      return new MutableLexicalFrame(mutability, /*initialCapacity=*/ numArgs);
+    }
   }
 
   private static final class ImmutableEmptyLexicalFrame implements LexicalFrame {
@@ -184,10 +189,16 @@ public String toString() {
   private static final class MutableLexicalFrame implements LexicalFrame {
     private final Mutability mutability;
     /** Bindings are maintained in order of creation. */
-    private final LinkedHashMap<String, Object> bindings = new LinkedHashMap<>();
+    private final LinkedHashMap<String, Object> bindings;
 
-    public MutableLexicalFrame(Mutability mutability) {
+    private MutableLexicalFrame(Mutability mutability, int initialCapacity) {
       this.mutability = mutability;
+      this.bindings = new LinkedHashMap<>(initialCapacity);
+    }
+
+    private MutableLexicalFrame(Mutability mutability) {
+      this.mutability = mutability;
+      this.bindings = new LinkedHashMap<>();
     }
 
     @Override

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

@@ -63,10 +63,12 @@ public Object call(Object[] arguments, FuncallExpression ast, Environment env)
               getName(), env.getCurrentFunction().getName()));
     }
 
-    Profiler.instance().startTask(ProfilerTask.SKYLARK_USER_FN, getName());
+    ImmutableList<String> names = signature.getSignature().getNames();
+    LexicalFrame lexicalFrame =
+        LexicalFrame.createForUserDefinedFunctionCall(env.mutability(), /*numArgs=*/ names.size());
     try {
-      env.enterScope(this, LexicalFrame.create(env.mutability()), ast, definitionGlobals);
-      ImmutableList<String> names = signature.getSignature().getNames();
+      Profiler.instance().startTask(ProfilerTask.SKYLARK_USER_FN, getName());
+      env.enterScope(this, lexicalFrame, ast, definitionGlobals);
 
       // Registering the functions's arguments as variables in the local Environment
       int i = 0;

src/test/java/com/google/devtools/build/lib/syntax/FunctionTest.java

@@ -199,6 +199,18 @@ public void testFunctionCallFromFunctionReadGlobalVar() throws Exception {
     assertThat(lookup("b")).isEqualTo(1);
   }
 
+  @Test
+  public void testFunctionParamCanShadowGlobalVarAfterGlobalVarIsRead() throws Exception {
+    eval("a = 1",
+        "def func2(a):",
+        "  return 0",
+        "def func1():",
+        "  dummy = a",
+        "  return func2(2)",
+        "b = func1()\n");
+    assertThat(lookup("b")).isEqualTo(0);
+  }
+
   @Test
   public void testSingleLineFunction() throws Exception {
     eval("def func(): return 'a'",