Handle closure calls more precisely in type flow analysis #39692
Assignees
Labels
area-vm
Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.
P2
A bug or feature request we're likely to work on
triaged
Issue has been triaged by sub team
type-performance
Issue relates to performance or code size
vm-aot-code-size
Related to improvements in AOT code size
vm-tfa
Comments
alexmarkov
added
area-vm
|
Victor noticed that after tear'ing off a static method the parameters no longer get inferred to be non-null. @victoragnez can you give alex an example of this? |
copybara-service bot
pushed a commit
that referenced
this issue
Aug 8, 2023
This is a step towards detaching bodies of closures/local functions from the enclosing functions in TFA, so closures could be analyzed separately. Also, this change allows TFA to accumulate types of captured local variables among all invocations. On a large Flutter app, this change doesn't cause noticeable compilation time or snapshot size changes. Issue: #39692 Issue: #51102 TEST=pkg/vm/testcases/transformations/type_flow/summary_collector/control_flow.dart Cq-Include-Trybots: luci.dart.try:vm-aot-linux-release-x64-try,vm-aot-mac-product-arm64-try Change-Id: I785b15656df559a8cc80fcceea196b480ba7a91a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/318021 Reviewed-by: Martin Kustermann <[email protected]> Commit-Queue: Alexander Markov <[email protected]>
copybara-service bot
pushed a commit
that referenced
this issue
Oct 12, 2023
This change adds propagation of closure values to TFA. For now, inferred closure values are used only when they are constant (tear-off of a static method or a constructor). Arbitrary closures can now be referenced from unrelated members via closure-id metadata. In addition, this change fixes an incorrect stack trace when an implicit closure (tear-off) was propagated and its call was inlined. Inlining interval was not recorded because of the missing source position of a call to a target method within implicit closure. TEST=pkg/vm/testcases/transformations/type_flow/transformer/closures.dart Issue: #39692 Change-Id: I3590da91b6057e0b55a8614382dba1bbcc267b39 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/325447 Commit-Queue: Alexander Markov <[email protected]> Reviewed-by: Martin Kustermann <[email protected]> Reviewed-by: Slava Egorov <[email protected]>
|
#53571 was closed because a solution to this issue is supposed to fix it. That issue contains a micro-benchmark that can then be used to verify this assumption. (this is a reminder to myself to check this once this issue has been fixed) |
alexmarkov
added
P2
copybara-service bot
pushed a commit
that referenced
this issue
Dec 1, 2023
dart2wasm lowering of async* methods includes call to Stream.where which takes a closure. The return type of that closure was incorrectly set to Object?, while it should be bool. This incorrect static type becomes a problem with more precise handling of closures in TFA. Issue: #39692 Change-Id: I35a8a6b198413d564e091a935a8beaff15d6d541 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/339200 Reviewed-by: Johnni Winther <[email protected]> Commit-Queue: Alexander Markov <[email protected]> Reviewed-by: Ömer Ağacan <[email protected]>
copybara-service bot
pushed a commit
that referenced
this issue
Dec 12, 2023
Instead of implicitly creating closures for FFI
asFunction/lookupFunction APIs in the VM, now they are explicitly expressed in the kernel AST. That makes it possible to analyze
them in TFA.
FFI calls from Dart code into native are now performed in the following way:
```
block {
Pointer #ffiTarget0 = target;
@pragma('vm:ffi:call-closure', _FfiCall<Int32 Function(Int32)>(isLeaf: false))
#ffiClosure0(int arg1) {
_nativeEffect(arg1);
return _ffiCall<int>(#ffiTarget0);
}
} =>#ffiClosure0;
```
_ffiCall method is recognized by the VM and its call is replaced
directly with FFI calling sequence. _ffiCall uses closure
parameters implicitly. No extra trampolines are generated for FFI calls.
TEST=existing
Fixes #54172
Issue #39692
CoreLibraryReviewExempt: Implementation change only.
Change-Id: I92b3ff7391470686151ad0807e2cdbbf1a69d256
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/339662
Commit-Queue: Alexander Markov <[email protected]>
Reviewed-by: Daco Harkes <[email protected]>
copybara-service bot
pushed a commit
that referenced
this issue
Dec 15, 2023
This reverts commit 1800039. The changes fd2e9b9 and c20f9ea are relanded as is. Reason for revert was fixed separately in https://dart-review.googlesource.com/c/sdk/+/341621 TEST=ci CoreLibraryReviewExempt: Implementation change only. Issue: #54172 Issue: #39692 Change-Id: I1a2324768502e5ffbce328127938c0d3c96c38ba Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/341642 Reviewed-by: Siva Annamalai <[email protected]> Commit-Queue: Alexander Markov <[email protected]>
copybara-service bot
pushed a commit
that referenced
this issue
Jan 16, 2024
This change adds analysis of function calls when closure target of a call can be inferred. Local functions are now analyzed separately from enclosing members. Inferred result type of function calls is now used in the AOT compiler. Time of AOT compilation step 2 (TFA) on a large Flutter application: Before: 59.448s After: 61.870s (+4%) Maintaining hierarchy of function types and analysis of all function calls is not feasible as it causes unbearable increase in AOT compilation time. TEST=existing Issue: #39692 Change-Id: Ieb4d5dce23868b5ab5c87fa1e77e49b85fd656fe Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345083 Reviewed-by: Slava Egorov <[email protected]> Reviewed-by: Martin Kustermann <[email protected]> Commit-Queue: Alexander Markov <[email protected]>
copybara-service bot
pushed a commit
that referenced
this issue
Jan 17, 2024
This reverts commit 66d3eed. Reason for revert: breaks google3 (b/320642692) Original change's description: > [vm/aot/tfa] Analyze function calls > > This change adds analysis of function calls when closure target > of a call can be inferred. Local functions are now analyzed separately > from enclosing members. Inferred result type of function calls is now > used in the AOT compiler. > > Time of AOT compilation step 2 (TFA) on a large Flutter application: > Before: 59.448s > After: 61.870s (+4%) > > Maintaining hierarchy of function types and analysis of all function > calls is not feasible as it causes unbearable increase in AOT > compilation time. > > TEST=existing > Issue: #39692 > > Change-Id: Ieb4d5dce23868b5ab5c87fa1e77e49b85fd656fe > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345083 > Reviewed-by: Slava Egorov <[email protected]> > Reviewed-by: Martin Kustermann <[email protected]> > Commit-Queue: Alexander Markov <[email protected]> Issue: #39692 Change-Id: Ieb9104f4263e19ef9e5bd749e935f6c2dbec3046 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/346780 Reviewed-by: Martin Kustermann <[email protected]> Reviewed-by: Slava Egorov <[email protected]> Bot-Commit: Rubber Stamper <[email protected]> Commit-Queue: Emmanuel Pellereau <[email protected]>
copybara-service bot
pushed a commit
that referenced
this issue
Jan 18, 2024
This is a reland of commit 66d3eed g3 breakage was fixed in https://dart-review.googlesource.com/c/sdk/+/346940. Original change's description: > [vm/aot/tfa] Analyze function calls > > This change adds analysis of function calls when closure target > of a call can be inferred. Local functions are now analyzed separately > from enclosing members. Inferred result type of function calls is now > used in the AOT compiler. > > Time of AOT compilation step 2 (TFA) on a large Flutter application: > Before: 59.448s > After: 61.870s (+4%) > > Maintaining hierarchy of function types and analysis of all function > calls is not feasible as it causes unbearable increase in AOT > compilation time. TEST=existing Issue: #39692 > Change-Id: Ieb4d5dce23868b5ab5c87fa1e77e49b85fd656fe > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345083 > Reviewed-by: Slava Egorov <[email protected]> > Reviewed-by: Martin Kustermann <[email protected]> > Commit-Queue: Alexander Markov <[email protected]> Change-Id: I05377121ffa7e56748c8a5ab58551e2b1caef70b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/346946 Commit-Queue: Slava Egorov <[email protected]> Reviewed-by: Martin Kustermann <[email protected]> Reviewed-by: Slava Egorov <[email protected]>
copybara-service bot
pushed a commit
that referenced
this issue
Mar 13, 2024
TFA now produces direct call metadata for function calls based on the results of the analysis. VM reads direct call metadata for function calls, but only uses it if target is a tear-off, as it cannot handle inlining of an arbitrary closure function yet (due to scope building dependencies between compilation of a closure and its parent function). TEST=pkg/vm/testcases/transformations/type_flow/transformer/closures.dart Issue: #39692 Change-Id: Idf0b6988534bfca1a64c6661a82df8d453272abf Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357180 Reviewed-by: Martin Kustermann <[email protected]> Reviewed-by: Slava Egorov <[email protected]> Commit-Queue: Alexander Markov <[email protected]>
|
@alexmarkov can this issue be closed as done ? |
|
@a-siva Yes, I think we can close this issue as done. We end up analyzing only monomorphic closure calls, because analyzing all possible closure calls results in a huge combinatorial explosion and increases AOT compilation time too much. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently, all closures are treated conservatively in TFA: closure body is included into enclosing member, and parameters are approximated using static types. Closure calls are not analyzed.
TFA can be improved to track closure calls and handle closure bodies separately from enclosing member.
The text was updated successfully, but these errors were encountered: