languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-10-02 23:39:19 +00:00
Code Issues Packages Projects Releases Wiki Activity

[flow analysis] Don't create a new empty map for every FlowModel

Browse source 
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>
This commit is contained in:
Jens Johansen 2023-05-04 06:25:28 +00:00 committed by Commit Queue
parent 8ea8160cd8
commit c0c9c98787
2 changed files with 34 additions and 60 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

25
pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart
View file

@ -1900,9 +1900,6 @@ class FlowModel<Type extends Object> {
/// [_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 @@ class FlowModel<Type extends Object> {
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 @@ class FlowModel<Type extends Object> {
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 @@ class FlowModel<Type extends Object> {
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 @@ class FlowModel<Type extends Object> {
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 @@ class FlowModel<Type extends Object> {
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 @@ class FlowModel<Type extends Object> {
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 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
}
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 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
}
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

69
pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart
View file

@ -4613,7 +4613,6 @@ main() {
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 @@ main() {
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 @@ main() {
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 @@ main() {
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 @@ main() {
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 @@ main() {
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 @@ main() {
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 @@ main() {
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 @@ main() {
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 @@ main() {
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 @@ main() {
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 @@ main() {
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 @@ main() {
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 @@ main() {
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 @@ main() {
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 @@ main() {
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 @@ main() {
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 @@ main() {
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);