Commit Graph

23 Commits

Author SHA1 Message Date
Clement Skau 12a5c93d4a Revert "Reland "[VM] Remove unnecessary Closure.clone() from sync*.""
This reverts commit 1bf25798fa.

Reason for revert: Rolling back as it breaks more external targets and is currently blocking rolls.

Original change's description:
> Reland "[VM] Remove unnecessary Closure.clone() from sync*."
> 
> This reverts commit 430c75b5fa.
> 
> Reason for revert: We probably don't care about the dartkb breakages.
> 
> Original change's description:
> > Revert "[VM] Remove unnecessary Closure.clone() from sync*."
> > 
> > This reverts commit ca94ad7112.
> > 
> > Reason for revert: Appears to break dartkb.
> > 
> > Original change's description:
> > > [VM] Remove unnecessary Closure.clone() from sync*.
> > > 
> > > Instead, this CL wraps sync*'s sync_op in an additional function
> > > that captures local copies of the necessary closure state.
> > > Notably this state includes the parameters of the original sync* function
> > > as these also need to be cloned for each instance.
> > > 
> > > 
> > > Note: a large number of .expect files have a small number of changes
> > > due to the additional wrapper function, as well as some extra plumbing.
> > > 
> > > Bug: https://github.com/dart-lang/sdk/issues/37753
> > > Change-Id: If099dbfe9e5d62979626b852386fd0173fc76de5
> > > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/136190
> > > Commit-Queue: Clement Skau <cskau@google.com>
> > > Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
> > 
> > TBR=vegorov@google.com,cskau@google.com
> > 
> > Change-Id: I89e517196f03622337f909f0fcecbdc77c9dda93
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Bug: https://github.com/dart-lang/sdk/issues/37753
> > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/144920
> > Reviewed-by: Clement Skau <cskau@google.com>
> > Commit-Queue: Clement Skau <cskau@google.com>
> 
> TBR=vegorov@google.com,cskau@google.com
> 
> Change-Id: I78589c83cbb17151ca4ab8f811b4f18f252fb9c1
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: https://github.com/dart-lang/sdk/issues/37753
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/144921
> Reviewed-by: Clement Skau <cskau@google.com>
> Commit-Queue: Clement Skau <cskau@google.com>

TBR=vegorov@google.com,cskau@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: https://github.com/dart-lang/sdk/issues/37753
Change-Id: I879df17bd282d01468fa010b94a8c89afdb8cd0d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/145361
Reviewed-by: Clement Skau <cskau@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Clement Skau <cskau@google.com>
2020-04-29 09:52:30 +00:00
Clement Skau 1bf25798fa Reland "[VM] Remove unnecessary Closure.clone() from sync*."
This reverts commit 430c75b5fa.

Reason for revert: We probably don't care about the dartkb breakages.

Original change's description:
> Revert "[VM] Remove unnecessary Closure.clone() from sync*."
> 
> This reverts commit ca94ad7112.
> 
> Reason for revert: Appears to break dartkb.
> 
> Original change's description:
> > [VM] Remove unnecessary Closure.clone() from sync*.
> > 
> > Instead, this CL wraps sync*'s sync_op in an additional function
> > that captures local copies of the necessary closure state.
> > Notably this state includes the parameters of the original sync* function
> > as these also need to be cloned for each instance.
> > 
> > 
> > Note: a large number of .expect files have a small number of changes
> > due to the additional wrapper function, as well as some extra plumbing.
> > 
> > Bug: https://github.com/dart-lang/sdk/issues/37753
> > Change-Id: If099dbfe9e5d62979626b852386fd0173fc76de5
> > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/136190
> > Commit-Queue: Clement Skau <cskau@google.com>
> > Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
> 
> TBR=vegorov@google.com,cskau@google.com
> 
> Change-Id: I89e517196f03622337f909f0fcecbdc77c9dda93
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: https://github.com/dart-lang/sdk/issues/37753
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/144920
> Reviewed-by: Clement Skau <cskau@google.com>
> Commit-Queue: Clement Skau <cskau@google.com>

TBR=vegorov@google.com,cskau@google.com

Change-Id: I78589c83cbb17151ca4ab8f811b4f18f252fb9c1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: https://github.com/dart-lang/sdk/issues/37753
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/144921
Reviewed-by: Clement Skau <cskau@google.com>
Commit-Queue: Clement Skau <cskau@google.com>
2020-04-28 06:17:20 +00:00
Clement Skau 430c75b5fa Revert "[VM] Remove unnecessary Closure.clone() from sync*."
This reverts commit ca94ad7112.

Reason for revert: Appears to break dartkb.

Original change's description:
> [VM] Remove unnecessary Closure.clone() from sync*.
> 
> Instead, this CL wraps sync*'s sync_op in an additional function
> that captures local copies of the necessary closure state.
> Notably this state includes the parameters of the original sync* function
> as these also need to be cloned for each instance.
> 
> 
> Note: a large number of .expect files have a small number of changes
> due to the additional wrapper function, as well as some extra plumbing.
> 
> Bug: https://github.com/dart-lang/sdk/issues/37753
> Change-Id: If099dbfe9e5d62979626b852386fd0173fc76de5
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/136190
> Commit-Queue: Clement Skau <cskau@google.com>
> Reviewed-by: Vyacheslav Egorov <vegorov@google.com>

TBR=vegorov@google.com,cskau@google.com

Change-Id: I89e517196f03622337f909f0fcecbdc77c9dda93
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: https://github.com/dart-lang/sdk/issues/37753
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/144920
Reviewed-by: Clement Skau <cskau@google.com>
Commit-Queue: Clement Skau <cskau@google.com>
2020-04-27 07:45:04 +00:00
Clement Skau ca94ad7112 [VM] Remove unnecessary Closure.clone() from sync*.
Instead, this CL wraps sync*'s sync_op in an additional function
that captures local copies of the necessary closure state.
Notably this state includes the parameters of the original sync* function
as these also need to be cloned for each instance.


Note: a large number of .expect files have a small number of changes
due to the additional wrapper function, as well as some extra plumbing.

Bug: https://github.com/dart-lang/sdk/issues/37753
Change-Id: If099dbfe9e5d62979626b852386fd0173fc76de5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/136190
Commit-Queue: Clement Skau <cskau@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
2020-04-27 07:37:39 +00:00
Alexander Markov fa54a92970 [vm/nnbd] NNBD fixes for bytecode
* Take nullability of type parameters into account.
* Handle CheckNull instruction with null selector name.

Fixes https://github.com/dart-lang/sdk/issues/41162
Fixes https://github.com/dart-lang/sdk/issues/41163

Change-Id: I0ff4c2790ea63e8f23ead6c9f479b8806045f012
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/140800
Reviewed-by: Régis Crelier <regis@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
2020-03-24 21:50:57 +00:00
Alexander Markov d850801691 [vm] Cleanup code related to interface calls through fields/getters from TFA and bytecode generator
Since https://dart-review.googlesource.com/c/sdk/+/129702 CFE desugars
interface calls through fields/getters, so there is no need to handle
them in the TFA and bytecode generator.

Note that VM can still see such calls if they are dynamic, so
handling of such calls is not removed entirely from TFA or VM.
Also, VM should support those as long as it supports older kernel
binary versions.

Issue: https://github.com/dart-lang/sdk/issues/34497
Change-Id: Ic49f109b0e9264f0e20a7e1a3b7a46011fa76c86
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/137700
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
2020-03-03 22:36:23 +00:00
Alexander Markov c74a8ec2c4 [vm/bytecode] Fix handling of contravariant parameters in isUncheckedCall
Fixes language_2/variance/variance_in_method_test.dart when bytecode is
enabled in AOT mode.

Change-Id: I2d1338f2d68ab3a91127cdbeb60c71597d4da718
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/127981
Reviewed-by: Régis Crelier <regis@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
2019-12-10 20:28:46 +00:00
Alexander Markov 5b5f34f2e6 [vm] Cleanup try-catch blocks around Expression.getStaticType
Expression.getStaticType was fixed in https://github.com/dart-lang/sdk/commit/970c164826d67fc2cd29ff596e5a103e9ee05ff2
Now we can remove try-catch blocks around getStaticType.

Issue: https://github.com/dart-lang/sdk/issues/38597
Issue: https://github.com/dart-lang/sdk/issues/34496
Change-Id: Ib68d6203875b8731fedf7017347004b06fad19de
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/125996
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
2019-11-22 16:48:04 +00:00
Régis Crelier af541cd0dd [VM/nnbd] Ignore nullability of type Never provided by CFE.
Although the type Never is always non-nullable, the CFE writes a nullability
byte in kernel files that was not properly skipped by the VM.

This fixes issues
https://github.com/dart-lang/sdk/issues/39466
and
https://github.com/dart-lang/sdk/issues/39461.

Change-Id: I93c44ed99173789fb19ddf9995f440d08e1c944b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/125929
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Régis Crelier <regis@google.com>
2019-11-21 21:04:15 +00:00
Johnni Winther 4b5589e0cb Reland "[cfe] Use StaticTypeContext for getStaticType" and more"
This relands commits
 "[cfe] Use StaticTypeContext for getStaticType"
 45033c6ad9

 "[cfe] Move caching of thisType to CoreTypes"
 fab25cbe0c

 "[vm/bytecode] Notify static type context when entering/leaving library"
 c9f88ae253

Change-Id: I407d6b0e3b3df503cedc9fcb2c834d835cf083f8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/125343
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
2019-11-16 05:50:58 +00:00
Johnni Winther ef5fb7d3f4 Revert "[cfe] Use StaticTypeContext for getStaticType" and more
This reverts commits
 "[cfe] Use StaticTypeContext for getStaticType"
 45033c6ad9

 "[cfe] Move caching of thisType to CoreTypes"
 fab25cbe0c

 "[vm/bytecode] Notify static type context when entering/leaving library"
 c9f88ae253

Change-Id: I6171f6a74b97f2f4a77a36c242330fc2db94c82f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/125341
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Aart Bik <ajcbik@google.com>
Auto-Submit: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Aart Bik <ajcbik@google.com>
2019-11-14 22:51:43 +00:00
Johnni Winther 45033c6ad9 [cfe] Use StaticTypeContext for getStaticType
Change Expression.getStaticType to take a StaticTypeContext instead
of a TypeEnvironment.

The StaticTypeContext provides access to the TypeEnvironment and the
current 'this type' as well as determining the nullability state of the
enclosing library.

This change is needed to support nnbd types for getStaticType and also
serves as a step towards supporting caching during static type
computations to avoid repeated computations of the same (complex)
expressions.

Change-Id: Ied974dff7f6f7c3c8f262aa80c8dea5c674662f7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/124683
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Dmitry Stefantsov <dmitryas@google.com>
2019-11-14 09:09:26 +00:00
Dmitry Stefantsov b4554c1215 [cfe] Make nullability a required parameter for types
Change-Id: Id787291294cafb15cf4ecad4a9db9da4c6066638
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/124135
Commit-Queue: Dmitry Stefantsov <dmitryas@google.com>
Auto-Submit: Dmitry Stefantsov <dmitryas@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
2019-11-07 13:24:47 +00:00
Alexander Markov 2ca6bc29b3 [vm,bytecode] More efficient bytecode for calls through fields/getters
Previously, calls through field/getter were generated as dynamic calls
because InterfaceCall with field/getter target doesn't correctly
represent what such call should do. This results in a quite inefficient
code in AOT mode.

With this change, calls through fields/getters are decomposed into two
calls, while keeping the same order of evaluation.

Change-Id: Ieb33927c1b39d1b4d8eafb7bd542675820474068
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/120628
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Régis Crelier <regis@google.com>
2019-10-08 22:27:10 +00:00
Alexander Markov 9fda4c1101 [vm/bytecode] Avoid using closure call instructions if code coverage may be needed
This is a workaround to https://github.com/dart-lang/sdk/issues/37944.
Closure call instructions are not tracked by code coverage, so this change
avoids generating them in bytecode if incremental kernel compiler is used -
when vm-service is enabled with command-line option and code coverage
may be needed.

Change-Id: Ifb3bc5e90a9f8ccb04073994d761bc301bc274ab
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/114045
Reviewed-by: Régis Crelier <regis@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
2019-08-22 00:28:34 +00:00
Alexander Markov 513ef1dfe1 [vm/bytecode] Avoid using map().toList() in bytecode generator
Code map((x) => ...).toList() is less efficient compared to an explicit loop.
Replacing these code patterns with loops slightly improves bytecode generation
speed (dart2js 1271 -> 1188 ms).

Also, map with closure consumes more stack space which results in flaky
stack overflow errors in kernel service on a test with deeply nested constants.

Change-Id: If209fc28d521abcbad46b5b2c7cffefcf8cd0ecb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/113217
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Régis Crelier <regis@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
2019-08-15 22:15:02 +00:00
Alexander Markov ac45ee7e63 [vm/bytecode] Support exactness tracking in bytecode
MicroClosureCreateTearoffClassSecondTime(RunTime) 3922 us -> 2030 us.

Issue: https://github.com/dart-lang/sdk/issues/36429
Change-Id: I3a6f93950a469969d82243fd1744d5a4f7fb1e78
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/111868
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
2019-08-03 17:36:31 +00:00
Alexander Markov 0a2fb22230 [vm/bytecode] Use simpleInstanceOf for rare types in bytecode
Corresponding change in the kernel AST flow graph builder:
https://dart-review.googlesource.com/c/sdk/+/110566.

Change-Id: Ia7c7d16824dc03eb0980b0b8aa28aa882d70c5b2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/110741
Reviewed-by: Régis Crelier <regis@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
2019-07-29 17:30:16 +00:00
Alexander Markov f450cf399e [vm/bytecode] Add UncheckedClosureCall instruction
This improves performance of closure calls:
DeltaBlueClosures: +25.14%
DartMicroBenchMM.MaxLib +40.91%
DartMicroBenchMM.MinLib +42.58%

Change-Id: I13000e20b9433ce77bb2ba5209513de575656036
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/109220
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Régis Crelier <regis@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
2019-07-17 21:05:44 +00:00
Alexander Markov da8cb470cc [vm/bytecode] Represent classes, libraries, scripts and recursive types in bytecode
Size of a large application:
Before: 26628600
After: 21480120 (-19.3%)

Size of snapshots:

isolate_snapshot_framework.bin
Before: 9322496
After: 6782976 (-27.2%)

isolate_snapshot_product_framework.bin
Before: 9166848
After: 6602752 (-27.9%)

Regressions in tests:

1) Test language_2/type_alias_equality_test/04 fails similarly to default mode, as VM
does not implement comparison of function types according to the specification.
Previously this test was passing as function types were canonicalized in bytecode,
which was not always correct. This CL fixes the problem with canonicalization of
function types in bytecode and the test starts failing again.

2) Tests standalone_2/entrypoints_verification_test, standalone_2/io/test_extension_test,
standalone_2/io/test_extension_fail_test fail as native extensions are not supported
in bytecode yet. These tests start passing after df5e7aac17,
which switched bytecode tests to kernel service (on x64), because kernel service doesn't
drop ASTs. This CL switches from reading AST library declarations to bytecode even if
AST is not removed, so tests fail again.

Change-Id: I8b7ba44bfa49d0b1599b2509553ff7c831a4e244
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/104700
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Régis Crelier <regis@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
2019-06-20 18:27:21 +00:00
Alexander Markov ebd13e891f [vm/bytecode] Add UncheckedInterfaceCall instruction
dart --use-bytecode-compiler NavierStokes.dart
Before: NavierStokes(RunTime): 8909.053017777778 us.
After: NavierStokes(RunTime): 7221.510314079423 us.

Issue: https://github.com/dart-lang/sdk/issues/36429
Issue: https://github.com/dart-lang/sdk/issues/36428

Change-Id: Ib1be4dd20cdc25e3b979a91a098f67a7bc00df8a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/98945
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Régis Crelier <regis@google.com>
2019-04-10 21:49:23 +00:00
Alexander Markov 61f0f5bc43 [vm/bytecode] Declare members in bytecode
This change replaces kernel AST declarations of fields and functions
with bytecode declarations.

Size of dilp files is reduced by 11-12%.

Startup latency:
Time to the first full frame: 1.945s -> 1.687s
FinalizeClass: 554ms -> 277ms
FinishClassLoading: 296ms -> 156ms

There are following regressions in bytecode mode, which will be fixed
in future:

* dart:mirrors are not supported yet (implementation of mirrors relies
  on reading kernel AST in certain cases).

  As the result, lib_2/mirrors/* tests fail.

* native extensions are not supported yet (annotations on libraries
  and classes in AST are cleaned up as they could reference members
  which are now removed from AST).

  As the result, standalone_2/entrypoints_verification_test test fails.

* language_2/spread_collections/const_error_test/* tests fail
  due to https://github.com/dart-lang/sdk/issues/36286.

Change-Id: I5130f401fd7b84038b136136e7ccc1a6e51b6cea
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/97561
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
2019-03-26 21:40:10 +00:00
Alexander Markov 336969538c [vm/bytecode] Revise representation of types, constants and calls in bytecode
* Type arguments are moved to global object table (and de-duplicated).
* Finalize non-recursive generic types at bytecode generation time.
* Constants are moved to global object table (and de-duplicated).
* ICData creation is avoided for direct calls.
* Interface calls reference target member instead of selector name.

Size of a dilp files: 17868K => 15896K (-11%).

Change-Id: I0c9bf338137a0fae6ed90ab6b125ed2b24a1a8ad
Reviewed-on: https://dart-review.googlesource.com/c/91108
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Zach Anderson <zra@google.com>
Reviewed-by: Régis Crelier <regis@google.com>
2019-02-05 20:13:45 +00:00