Skip to content

Commit 318a482

Browse files
alexmarkovcommit-bot@chromium.org
authored andcommitted
[vm/aot/tfa] Improve handling of recursive calls in TFA
In general case, TFA approximates results of recursive calls using static types. However, if result type of a function does not depend on the flow inside its body, it cannot change and it can be used in case of recursive calls instead of a static type. This improves micro-benchmark from #37455: Before: 0m11.506s After: 0m7.324s Issue: #37455 Change-Id: I967d7add906c8dbd59dbbea1b993e1b4e1733514 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/108500 Commit-Queue: Alexander Markov <[email protected]> Reviewed-by: Martin Kustermann <[email protected]>
1 parent 016061d commit 318a482

File tree

4 files changed

+124
-11
lines changed

4 files changed

+124
-11
lines changed

pkg/vm/lib/transformations/type_flow/analysis.dart

+44-11
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,14 @@ abstract class _Invocation extends _DependencyTracker
9898
/// its result is saturated in order to guarantee convergence.
9999
static const int invalidationLimit = 1000;
100100

101+
_Invocation(this.selector, this.args);
102+
101103
Type process(TypeFlowAnalysis typeFlowAnalysis);
102104

103-
_Invocation(this.selector, this.args);
105+
/// Returns result of this invocation if its available without
106+
/// further analysis, or `null` if it's not available.
107+
/// Used for recursive calls while this invocation is being processed.
108+
Type get resultForRecursiveInvocation => result;
104109

105110
// Only take selector and args into account as _Invocation objects
106111
// are cached in _InvocationsCache using selector and args as a key.
@@ -232,9 +237,17 @@ class _DirectInvocation extends _Invocation {
232237
final Member member = selector.member;
233238
if (selector.memberAgreesToCallKind(member)) {
234239
if (_argumentsValid()) {
235-
return typeFlowAnalysis
236-
.getSummary(member)
237-
.apply(args, typeFlowAnalysis.hierarchyCache, typeFlowAnalysis);
240+
final summary = typeFlowAnalysis.getSummary(member);
241+
// If result type is known upfront (doesn't depend on the flow),
242+
// set it eagerly so recursive invocations are able to use it.
243+
final summaryResult = summary.result;
244+
if (summaryResult is Type &&
245+
!typeFlowAnalysis.workList._isPending(this)) {
246+
assertx(result == null || result == summaryResult);
247+
result = summaryResult;
248+
}
249+
return summary.apply(
250+
args, typeFlowAnalysis.hierarchyCache, typeFlowAnalysis);
238251
} else {
239252
assertx(selector.callKind == CallKind.Method);
240253
return _processNoSuchMethod(args.receiver, typeFlowAnalysis);
@@ -301,6 +314,7 @@ class _DispatchableInvocation extends _Invocation {
301314
bool _isPolymorphic = false;
302315
Set<Call> _callSites; // Populated only if not polymorphic.
303316
Member _monomorphicTarget;
317+
_DirectInvocation _monomorphicDirectInvocation;
304318

305319
@override
306320
set typeChecksNeeded(bool value) {
@@ -368,6 +382,11 @@ class _DispatchableInvocation extends _Invocation {
368382
final directInvocation = typeFlowAnalysis._invocationsCache
369383
.getInvocation(directSelector, directArgs);
370384

385+
if (!_isPolymorphic) {
386+
assertx(target == _monomorphicTarget);
387+
_monomorphicDirectInvocation = directInvocation;
388+
}
389+
371390
type = typeFlowAnalysis.workList.processInvocation(directInvocation);
372391
if (kPrintTrace) {
373392
tracePrint('Dispatch: $directInvocation, result: $type');
@@ -561,10 +580,6 @@ class _DispatchableInvocation extends _Invocation {
561580
}
562581

563582
void addCallSite(Call callSite) {
564-
if (selector is DirectSelector) {
565-
return;
566-
}
567-
568583
_notifyCallSite(callSite);
569584
if (!callSite.isPolymorphic) {
570585
(_callSites ??= new Set<Call>()).add(callSite);
@@ -574,8 +589,6 @@ class _DispatchableInvocation extends _Invocation {
574589
/// Notify call site about changes in polymorphism or checkedness of this
575590
/// invocation.
576591
void _notifyCallSite(Call callSite) {
577-
assert(selector is! DirectSelector);
578-
579592
if (_isPolymorphic) {
580593
callSite.setPolymorphic();
581594
} else {
@@ -596,6 +609,17 @@ class _DispatchableInvocation extends _Invocation {
596609
_callSites.forEach(_notifyCallSite);
597610
}
598611
}
612+
613+
@override
614+
Type get resultForRecursiveInvocation {
615+
if (result != null) {
616+
return result;
617+
}
618+
if (_monomorphicDirectInvocation != null) {
619+
return _monomorphicDirectInvocation.resultForRecursiveInvocation;
620+
}
621+
return null;
622+
}
599623
}
600624

601625
/// Efficient builder of receiver type.
@@ -1313,7 +1337,16 @@ class _WorkList {
13131337
}
13141338
return result;
13151339
} else {
1316-
// Recursive invocation, approximate with static type.
1340+
// Recursive invocation.
1341+
final result = invocation.resultForRecursiveInvocation;
1342+
if (result != null) {
1343+
if (kPrintTrace) {
1344+
tracePrint("Already known type for recursive invocation: $result");
1345+
tracePrint('END PROCESSING $invocation, RESULT $result');
1346+
}
1347+
return result;
1348+
}
1349+
// Fall back to static type.
13171350
Statistics.recursiveInvocationsApproximated++;
13181351
final staticType =
13191352
new Type.fromStatic(invocation.selector.staticReturnType);

pkg/vm/lib/transformations/type_flow/summary_collector.dart

+14
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,20 @@ class _SummaryNormalizer extends StatementVisitor {
9696
return const EmptyType();
9797
} else if (n == 1) {
9898
return st.values.single;
99+
} else {
100+
final first = st.values.first;
101+
if (first is Type) {
102+
bool allMatch = true;
103+
for (int i = 1; i < n; ++i) {
104+
if (first != st.values[i]) {
105+
allMatch = false;
106+
break;
107+
}
108+
}
109+
if (allMatch) {
110+
return first;
111+
}
112+
}
99113
}
100114
}
101115

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// Regression test for https://github.com/dart-lang/sdk/issues/37455
6+
// Verifies that TFA can infer type of a recursive call if it doesn't depend
7+
// on the flow.
8+
9+
class A {
10+
// Should be inferred as _GrowableList.
11+
final List afield;
12+
13+
A(this.afield);
14+
15+
String toString() => afield.toString();
16+
}
17+
18+
class B {
19+
List _foo(Iterator<int> iter) {
20+
List result = [];
21+
while (iter.moveNext()) {
22+
if (iter.current < 0) {
23+
return result;
24+
}
25+
// Do a recursive call with the same arguments.
26+
result.add(new A(_foo(iter)));
27+
}
28+
return result;
29+
}
30+
}
31+
32+
void main() {
33+
var list = new B()._foo([1, 2, 3].iterator);
34+
print(list);
35+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
library #lib;
2+
import self as self;
3+
import "dart:core" as core;
4+
5+
class A extends core::Object {
6+
[@vm.inferred-type.metadata=dart.core::_GrowableList<dynamic>] [@vm.procedure-attributes.metadata=hasDynamicUses:false,hasThisUses:false,hasNonThisUses:false,hasTearOffUses:false] final field core::List<dynamic> afield;
7+
constructor •([@vm.inferred-type.metadata=dart.core::_GrowableList<dynamic>] core::List<dynamic> afield) → self::A
8+
: self::A::afield = afield, super core::Object::•()
9+
;
10+
[@vm.procedure-attributes.metadata=hasDynamicUses:false,hasThisUses:false,hasTearOffUses:false] method toString() → core::String
11+
return [@vm.direct-call.metadata=dart.core::_GrowableList::toString] [@vm.inferred-type.metadata=!? (skip check)] [@vm.direct-call.metadata=#lib::A::afield] [@vm.inferred-type.metadata=dart.core::_GrowableList<dynamic>] this.{self::A::afield}.{core::Object::toString}();
12+
}
13+
class B extends core::Object {
14+
synthetic constructor •() → self::B
15+
: super core::Object::•()
16+
;
17+
[@vm.procedure-attributes.metadata=hasDynamicUses:false,hasTearOffUses:false] method _foo([@vm.inferred-type.metadata=dart._internal::ListIterator<dart.core::int>] core::Iterator<core::int> iter) → core::List<dynamic> {
18+
core::List<dynamic> result = <dynamic>[];
19+
while ([@vm.direct-call.metadata=dart._internal::ListIterator::moveNext] [@vm.inferred-type.metadata=dart.core::bool (skip check)] iter.{core::Iterator::moveNext}()) {
20+
if([@vm.inferred-type.metadata=dart.core::bool?] [@vm.direct-call.metadata=dart._internal::ListIterator::current] iter.{core::Iterator::current}.{core::num::<}(0)) {
21+
return result;
22+
}
23+
[@vm.call-site-attributes.metadata=receiverType:dart.core::List<dynamic>] [@vm.direct-call.metadata=dart.core::_GrowableList::add] [@vm.inferred-type.metadata=!? (skip check)] result.{core::List::add}(new self::A::•([@vm.direct-call.metadata=#lib::B::_foo] [@vm.inferred-type.metadata=dart.core::_GrowableList<dynamic> (skip check)] this.{self::B::_foo}(iter)));
24+
}
25+
return result;
26+
}
27+
}
28+
static method main() → void {
29+
core::List<dynamic> list = [@vm.direct-call.metadata=#lib::B::_foo] [@vm.inferred-type.metadata=dart.core::_GrowableList<dynamic> (skip check)] new self::B::•().{self::B::_foo}([@vm.direct-call.metadata=dart.core::_GrowableList::iterator] [@vm.inferred-type.metadata=dart._internal::ListIterator<dart.core::int>]<core::int>[1, 2, 3].{core::Iterable::iterator});
30+
core::print(list);
31+
}

0 commit comments

Comments
 (0)