aboutsummaryrefslogtreecommitdiffhomepage
path: root/src/main/java/com
diff options
context:
space:
mode:
authorGravatar nharmata <nharmata@google.com>2018-03-08 16:43:45 -0800
committerGravatar Copybara-Service <copybara-piper@google.com>2018-03-08 16:45:57 -0800
commit62678a40664c9a43d3a05334156a3c6143127ed2 (patch)
tree443cec273b810b054818331a1699090e18c3bff4 /src/main/java/com
parent55a748bb2469ecc9be40c525d3c0a979d75a0670 (diff)
Optimize GC churn of parameter bindings performed before each user defined function call.
This is the 2nd attempt at this commit. The first attempt (https://github.com/bazelbuild/bazel/commit/f1013485d41efd8503f9d4f937e17d1b4bc91ed3) 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
Diffstat (limited to 'src/main/java/com')
-rw-r--r--src/main/java/com/google/devtools/build/lib/syntax/Environment.java15
-rw-r--r--src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java8
2 files changed, 18 insertions, 5 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
index d4e6403312..eb06a10862 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
@@ -142,6 +142,11 @@ public final class Environment implements Freezable {
? 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 final class Environment implements Freezable {
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
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
index b4fc95f5aa..485bac26e8 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
@@ -63,10 +63,12 @@ public class UserDefinedFunction extends BaseFunction {
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;