[flow analysis] Don't create a new empty map for every FlowModel
Currently every FlowModel creates a new empty map that's just supposed
to be empty. When compiling `compile.dart` (from the CFE) this creates
more than 200,000 maps for seemingly no reason.
This CL removes it, thus saving the creation of (...instrumenting the
platform...) 229,472 maps when compiling `compile.dart` (from the CFE).
Thinking it was done to avoid some polymorphism I have gone over the
created flowgraphs for the file in both JIT and AOT and found only
improvements.
AOT compiled `compile.dart` then compiling itself improves by ~3.5%:
```
Difference at 95.0% confidence
-0.1365 +/- 0.0576111
-3.55191% +/- 1.49912%
(Student's t, pooled s = 0.090011)
```
Change-Id: Ifdbb57e9aa3c23b2af512a2104aaf6caf72831ef
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/301061
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
diff --git a/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart b/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart
index 1691a69..d277196 100644
--- a/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart
+++ b/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart
@@ -1900,9 +1900,6 @@
/// [_FlowAnalysisImpl._promotionKeyStore].
final Map<int, VariableModel<Type> /*!*/ > variableInfo;
- /// The empty map, used to [join] variables.
- final Map<int, VariableModel<Type>> _emptyVariableMap = {};
-
/// Creates a state object with the given [reachable] status. All variables
/// are assumed to be unpromoted and already assigned, so joining another
/// state with this one will have no effect on it.
@@ -2493,7 +2490,6 @@
TypeOperations<Type> typeOperations,
FlowModel<Type>? first,
FlowModel<Type>? second,
- Map<int, VariableModel<Type>> emptyVariableMap,
) {
if (first == null) return second!;
if (second == null) return first;
@@ -2511,10 +2507,7 @@
Reachability newReachable =
Reachability.join(first.reachable, second.reachable);
Map<int, VariableModel<Type>> newVariableInfo = FlowModel.joinVariableInfo(
- typeOperations,
- first.variableInfo,
- second.variableInfo,
- emptyVariableMap);
+ typeOperations, first.variableInfo, second.variableInfo);
return FlowModel._identicalOrNew(
first, second, newReachable, newVariableInfo);
@@ -2526,13 +2519,13 @@
TypeOperations<Type> typeOperations,
Map<int, VariableModel<Type>> first,
Map<int, VariableModel<Type>> second,
- Map<int, VariableModel<Type>> emptyMap,
) {
if (identical(first, second)) return first;
if (first.isEmpty || second.isEmpty) {
- return emptyMap;
+ return const {};
}
+ // TODO(jensj): How often is this empty?
Map<int, VariableModel<Type>> result = <int, VariableModel<Type>>{};
bool alwaysFirst = true;
bool alwaysSecond = true;
@@ -2552,7 +2545,7 @@
if (alwaysFirst) return first;
if (alwaysSecond && result.length == second.length) return second;
- if (result.isEmpty) return emptyMap;
+ if (result.isEmpty) return const {};
return result;
}
@@ -2562,7 +2555,6 @@
TypeOperations<Type> typeOperations,
FlowModel<Type>? first,
FlowModel<Type>? second,
- Map<int, VariableModel<Type>> emptyVariableMap,
) {
if (first == null) return second!.unsplit();
if (second == null) return first.unsplit();
@@ -2580,10 +2572,7 @@
Reachability newReachable =
Reachability.join(first.reachable, second.reachable).unsplit();
Map<int, VariableModel<Type>> newVariableInfo = FlowModel.joinVariableInfo(
- typeOperations,
- first.variableInfo,
- second.variableInfo,
- emptyVariableMap);
+ typeOperations, first.variableInfo, second.variableInfo);
return FlowModel._identicalOrNew(
first, second, newReachable, newVariableInfo);
@@ -5153,7 +5142,7 @@
}
FlowModel<Type> _join(FlowModel<Type>? first, FlowModel<Type>? second) =>
- FlowModel.join(operations, first, second, _current._emptyVariableMap);
+ FlowModel.join(operations, first, second);
/// Creates a promotion key representing a temporary variable that doesn't
/// correspond to any variable in the user's source code. This is used by
@@ -5174,7 +5163,7 @@
}
FlowModel<Type> _merge(FlowModel<Type> first, FlowModel<Type>? second) =>
- FlowModel.merge(operations, first, second, _current._emptyVariableMap);
+ FlowModel.merge(operations, first, second);
/// Computes an updated flow model representing the result of a null check
/// performed by a pattern. The returned flow model represents what is known
diff --git a/pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart b/pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart
index 0ea0ece..413722c 100644
--- a/pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart
+++ b/pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart
@@ -4613,7 +4613,6 @@
var intType = Type('int');
var intQType = Type('int?');
var stringType = Type('String');
- const emptyMap = const <int, VariableModel<Type>>{};
setUp(() {
x = h.promotionKeyStore.keyForVariable(Var('x')..type = Type('Object?'));
@@ -4641,7 +4640,7 @@
x: model(null),
y: model([intType])
};
- expect(FlowModel.joinVariableInfo(h.typeOperations, p1, p2, emptyMap), {
+ expect(FlowModel.joinVariableInfo(h.typeOperations, p1, p2), {
x: _matchVariableModel(chain: null, ofInterest: ['int']),
y: _matchVariableModel(chain: null, ofInterest: ['int'])
});
@@ -4653,8 +4652,7 @@
x: model([intType]),
y: model([stringType])
};
- expect(FlowModel.joinVariableInfo(h.typeOperations, p, p, emptyMap),
- same(p));
+ expect(FlowModel.joinVariableInfo(h.typeOperations, p, p), same(p));
});
test('one input empty', () {
@@ -4663,10 +4661,11 @@
y: model([stringType])
};
var p2 = <int, VariableModel<Type>>{};
- expect(FlowModel.joinVariableInfo(h.typeOperations, p1, p2, emptyMap),
- same(emptyMap));
- expect(FlowModel.joinVariableInfo(h.typeOperations, p2, p1, emptyMap),
- same(emptyMap));
+ const expected = const <int, VariableModel<Never>>{};
+ expect(FlowModel.joinVariableInfo(h.typeOperations, p1, p2),
+ same(expected));
+ expect(FlowModel.joinVariableInfo(h.typeOperations, p2, p1),
+ same(expected));
});
test('promoted with unpromoted', () {
@@ -4677,10 +4676,8 @@
var expected = {
x: _matchVariableModel(chain: null, ofInterest: ['int'])
};
- expect(FlowModel.joinVariableInfo(h.typeOperations, p1, p2, emptyMap),
- expected);
- expect(FlowModel.joinVariableInfo(h.typeOperations, p2, p1, emptyMap),
- expected);
+ expect(FlowModel.joinVariableInfo(h.typeOperations, p1, p2), expected);
+ expect(FlowModel.joinVariableInfo(h.typeOperations, p2, p1), expected);
});
test('related type chains', () {
@@ -4693,10 +4690,8 @@
var expected = {
x: _matchVariableModel(chain: ['int?'], ofInterest: ['int?', 'int'])
};
- expect(FlowModel.joinVariableInfo(h.typeOperations, p1, p2, emptyMap),
- expected);
- expect(FlowModel.joinVariableInfo(h.typeOperations, p2, p1, emptyMap),
- expected);
+ expect(FlowModel.joinVariableInfo(h.typeOperations, p1, p2), expected);
+ expect(FlowModel.joinVariableInfo(h.typeOperations, p2, p1), expected);
});
test('unrelated type chains', () {
@@ -4709,10 +4704,8 @@
var expected = {
x: _matchVariableModel(chain: null, ofInterest: ['String', 'int'])
};
- expect(FlowModel.joinVariableInfo(h.typeOperations, p1, p2, emptyMap),
- expected);
- expect(FlowModel.joinVariableInfo(h.typeOperations, p2, p1, emptyMap),
- expected);
+ expect(FlowModel.joinVariableInfo(h.typeOperations, p1, p2), expected);
+ expect(FlowModel.joinVariableInfo(h.typeOperations, p2, p1), expected);
});
test('sub-map', () {
@@ -4722,10 +4715,8 @@
y: model([stringType])
};
var p2 = {x: xModel};
- expect(FlowModel.joinVariableInfo(h.typeOperations, p1, p2, emptyMap),
- same(p2));
- expect(FlowModel.joinVariableInfo(h.typeOperations, p2, p1, emptyMap),
- same(p2));
+ expect(FlowModel.joinVariableInfo(h.typeOperations, p1, p2), same(p2));
+ expect(FlowModel.joinVariableInfo(h.typeOperations, p2, p1), same(p2));
});
test('sub-map with matched subtype', () {
@@ -4739,10 +4730,8 @@
var expected = {
x: _matchVariableModel(chain: ['int?'], ofInterest: ['int?', 'int'])
};
- expect(FlowModel.joinVariableInfo(h.typeOperations, p1, p2, emptyMap),
- expected);
- expect(FlowModel.joinVariableInfo(h.typeOperations, p2, p1, emptyMap),
- expected);
+ expect(FlowModel.joinVariableInfo(h.typeOperations, p1, p2), expected);
+ expect(FlowModel.joinVariableInfo(h.typeOperations, p2, p1), expected);
});
test('sub-map with mismatched subtype', () {
@@ -4756,10 +4745,8 @@
var expected = {
x: _matchVariableModel(chain: ['int?'], ofInterest: ['int?', 'int'])
};
- expect(FlowModel.joinVariableInfo(h.typeOperations, p1, p2, emptyMap),
- expected);
- expect(FlowModel.joinVariableInfo(h.typeOperations, p2, p1, emptyMap),
- expected);
+ expect(FlowModel.joinVariableInfo(h.typeOperations, p1, p2), expected);
+ expect(FlowModel.joinVariableInfo(h.typeOperations, p2, p1), expected);
});
test('assigned', () {
@@ -4767,8 +4754,7 @@
var assigned = model(null, assigned: true);
var p1 = {x: assigned, y: assigned, z: unassigned, w: unassigned};
var p2 = {x: assigned, y: unassigned, z: assigned, w: unassigned};
- var joined =
- FlowModel.joinVariableInfo(h.typeOperations, p1, p2, emptyMap);
+ var joined = FlowModel.joinVariableInfo(h.typeOperations, p1, p2);
expect(joined, {
x: same(assigned),
y: _matchVariableModel(
@@ -4794,8 +4780,7 @@
z: writeCapturedModel,
w: intQModel
};
- var joined =
- FlowModel.joinVariableInfo(h.typeOperations, p1, p2, emptyMap);
+ var joined = FlowModel.joinVariableInfo(h.typeOperations, p1, p2);
expect(joined, {
x: same(writeCapturedModel),
y: same(writeCapturedModel),
@@ -4827,7 +4812,7 @@
test('first is null', () {
var s1 = FlowModel.withInfo(Reachability.initial.split(), emptyMap);
- var result = FlowModel.merge(h.typeOperations, null, s1, emptyMap);
+ var result = FlowModel.merge(h.typeOperations, null, s1);
expect(result.reachable, same(Reachability.initial));
});
@@ -4835,7 +4820,7 @@
var splitPoint = Reachability.initial.split();
var afterSplit = splitPoint.split();
var s1 = FlowModel.withInfo(afterSplit, emptyMap);
- var result = FlowModel.merge(h.typeOperations, s1, null, emptyMap);
+ var result = FlowModel.merge(h.typeOperations, s1, null);
expect(result.reachable, same(splitPoint));
});
@@ -4848,7 +4833,7 @@
var s2 = FlowModel.withInfo(afterSplit, {
x: varModel([stringType])
});
- var result = FlowModel.merge(h.typeOperations, s1, s2, emptyMap);
+ var result = FlowModel.merge(h.typeOperations, s1, s2);
expect(result.reachable, same(splitPoint));
expect(result.variableInfo[x]!.promotedTypes, isNull);
});
@@ -4862,7 +4847,7 @@
var s2 = FlowModel.withInfo(afterSplit, {
x: varModel([stringType])
});
- var result = FlowModel.merge(h.typeOperations, s1, s2, emptyMap);
+ var result = FlowModel.merge(h.typeOperations, s1, s2);
expect(result.reachable, same(splitPoint));
expect(result.variableInfo, same(s2.variableInfo));
});
@@ -4876,7 +4861,7 @@
var s2 = FlowModel.withInfo(afterSplit.setUnreachable(), {
x: varModel([stringType])
});
- var result = FlowModel.merge(h.typeOperations, s1, s2, emptyMap);
+ var result = FlowModel.merge(h.typeOperations, s1, s2);
expect(result.reachable, same(splitPoint));
expect(result.variableInfo, same(s1.variableInfo));
});
@@ -4890,7 +4875,7 @@
var s2 = FlowModel.withInfo(afterSplit.setUnreachable(), {
x: varModel([stringType])
});
- var result = FlowModel.merge(h.typeOperations, s1, s2, emptyMap);
+ var result = FlowModel.merge(h.typeOperations, s1, s2);
expect(result.reachable.locallyReachable, false);
expect(result.reachable.parent, same(splitPoint.parent));
expect(result.variableInfo[x]!.promotedTypes, isNull);
