On a flutter dill extracted via the mentioned bug I get these timings
when (once, i.e. I haven't done extra statistics) running
```
out/ReleaseX64/dart pkg/kernel/test/binary_bench2.dart --warmups=10 \
--iterations=5 --metadata AstFromBinaryEager <dill from flutter>
```
(and without `--metadata` for without metadata):
Without reading metadata: ~378 ms
With reading metadata before this CL: ~627 ms
With reading metadata with this CL: ~435 ms
Bug: https://github.com/flutter/flutter/issues/156713
Change-Id: Id6cb27bc00526ff61c48eeb66ebb86dff1b971a2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/391761
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This CL:
* Makes the incremental compiler copy over the metadata just created for
the libraries compiled into the Component returned.
* Preserves metadata when serializing via the incremental serializer.
* Fixes loading a multi-component with metadata.
* Lets the VM copy all metadata in "_combinePendingDeltas"
when there is only one component to "combine".
It is generally my view, that to do better, metadata has to be redone
(among other things likely has to live on the Library instead).
https://github.com/dart-lang/sdk/issues/39302
With this CL:
```
rm foo.dill ; tools/sdks/dart-sdk/bin/dart pkg/frontend_server/bin/frontend_server_starter.dart --no-print-incremental-dependencies --target=vm --sdk-root=out/ReleaseX64 --platform=vm_platform_strong.dill --output-dill=foo.dill --packages=.dart_tool/package_config.json pkg/compiler/lib/src/dart2js.dart
tools/sdks/dart-sdk/bin/dart pkg/vm/bin/dump_kernel.dart foo.dill foo.dump
grep "@vm.call-site-attributes" foo.dump | wc -l
```
(i.e. a one-shot compile) prints 9500.
```
rm foo.dill ; tools/sdks/dart-sdk/bin/dart pkg/frontend_server/bin/frontend_server_starter.dart --incremental --no-incremental-serialization --no-print-incremental-dependencies --target=vm --sdk-root=out/ReleaseX64 --platform=vm_platform_strong.dill --output-dill=foo.dill --packages=.dart_tool/package_config.json pkg/compiler/lib/src/dart2js.dart
tools/sdks/dart-sdk/bin/dart pkg/vm/bin/dump_kernel.dart foo.dill foo.dump
grep "@vm.call-site-attributes" foo.dump | wc -l
```
(i.e. an incremental compile but without incremental serialization) prints 9500.
and
```
rm foo.dill ; tools/sdks/dart-sdk/bin/dart pkg/frontend_server/bin/frontend_server_starter.dart --incremental --no-print-incremental-dependencies --target=vm --sdk-root=out/ReleaseX64 --platform=vm_platform_strong.dill --output-dill=foo.dill --packages=.dart_tool/package_config.json pkg/compiler/lib/src/dart2js.dart
tools/sdks/dart-sdk/bin/dart pkg/vm/bin/dump_kernel.dart foo.dill foo.dump
grep "@vm.call-site-attributes" foo.dump | wc -l
```
(i.e. an incremental compile with incremental serialization) prints 9500.
If not deleting the dill file first (which will then initialize from it)
metadata might be different though.
If there are no changes and the incremental serializer was and is used
we should still get `9530` (because the bytes are just copied), but
otherwise we might get different results because it's not really
possible to otherwise pick-and-choose and/or merge the metadata.
TEST=Existing tests that it didn't get worse.
Change-Id: I19d530357e7e2a174d7408dbbf0ed7e0248a5b70
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358444
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
The agnostic mode was added to allow the platform dill embedded in
the VM to support both weak and strong mode. Since weak mode is no
longer supported in the VM, the agnostic mode can new be deleted.
All uses of the agnostic in Dart and Flutter have been removed prior
to this change.
Change-Id: Iff0f69d9cd64e887e01cd7e7d336a97761bd6d4b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366801
Reviewed-by: Alexander Thomas <athom@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
This adds a `forErrorHandling` to the `Throw` node. This is used that the `throw` is *not* present in the source code but added to ensure
correctness and/or soundness of the generated code.
This is used for instance in the lowering for handling duplicate writes
to a late final field or for pattern assignments that don't match.
In response to https://github.com/dart-lang/sdk/issues/53519
TEST=updated ast-to-text
Change-Id: Ie103829d98fda9cd7b64e9e3d893e77d1e86d7d8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/347900
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Reviewed-by: Liam Appelbe <liama@google.com>
Generally the test loads a big dill thats 90+% the same content as the
previous load, then verifies it.
This CL loads smarter and verifies less.
Before this CL, locally, running
pkg/frontend_server/test/frontend_server_flutter.dart took
real 24m56.080s
user 48m42.422s
sys 1m2.360s
and the suite edition (using 4 shards in isolates) took
real 15m9.196s
user 53m41.118s
sys 1m30.045s
With this CL, locally running
pkg/frontend_server/test/frontend_server_flutter.dart takes
real 5m0.206s
user 9m23.933s
sys 0m20.984s
and the suite edition takes
real 3m24.243s
user 12m0.069s
sys 0m28.131s
On the try-bot the runtime seems to have gone from ~40 minutes
to ~20 minutes, the "compile flutter tests" step from ~30 minutes to
~10 minutes and the portion of time actually running the
dart-code that compiles, loads and verifies, from ~26 minutes
to ~7 minutes.
Change-Id: I6db225c33e1c0ee817f3880327e720446150ad7d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/347282
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
This adds an ìsImplicitCall to DynamicInvocation which is set on
expression like `d()` where `d` has type dynamic, to distinguish
the for `d.call()`.
TEST=pkg/front_end/testcases/general/dynamic_call.dart
Change-Id: I73beb911bdb315a510c862e6d4876cf7673ec3c7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/346240
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Previously we would encode the type of the value returned in `async`
functions as the field `futureValueType` on `FunctionNode`. For all
other kinds of functions, such as `sync`, `sync*`, and `async*`, that
field would be null. This CL renames `futureValueType` into
`emittedVAlueType`, and for functions of kinds `async`, `sync*`, and
`async*` that is expected to be the type of values emitted via
`return` or `yield` statements. For `sync` functions that field is
supposed to contain `null`.
In response to https://github.com/dart-lang/sdk/issues/54159
TEST=existing
Change-Id: I1efdbcc4e75d150f5618c7ca50cfe49a0e54fce6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/341662
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Ömer Ağacan <omersa@google.com>
Commit-Queue: Chloe Stefantsova <cstefantsova@google.com>
Reviewed-by: Mayank Patke <fishythefish@google.com>
This CL introduces an alternative way to calculate scopes for
expression evaluations. Currently the previous method is kept as well.
What this does is that it:
* Finds all nodes that match the offset and uri. (Ideally there's only
one, but that's not always the case --- it's the case for less than
60% of cases in the test added actually).
* Calculate the scope along the way.
* Return the scopes of all the found nodes that offset.
A test that asks for ~all file offsets in the dill files in the sdk and
compares the result with the scope the binary serialization computes is
added.
A single point can't always be found (for all dills in the sdk
only ~58.93% returns a single result), but for each query the wanted
scope is among the results returned.
For the non-outline dills in the sdk, between ~87.93% and ~94.52% of
the results are either a single result or multiple results with the
same actual scope in all of them.
Note that the test asks for ~all offsets, not just "debuggable" or
"stopable" offsets (which will likely vary depending on the vm/web
backend etc) --- likely the percentage will be higher for those points
though.
When this returns multiple results one can likely be picked by the
client based on information it has about names of variables in scope
etc.
Note that the test is rather slow, on my machine with the platforms
available there it makes 3,908,181 queries in ~3.5 minutes.
(Which corresponds to roughly 18,610 per second or under 0.06 ms per
query on average though, so it doesn't seem like a concert for actually
using it.)
Change-Id: I40b5360fcd935c70629543737e89a787de36ca16
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/332204
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
Expressions missing offsets were:
- LogicalExpression
- ConditionalExpression
- Not
- BlockExpression
- Instantiation
- TypedefTearOff*
* I'm not sure how TypedefTearOff is actually being used, I don't see reference to it in the VM runtime. Unless the CFE lowers it before the VM gets it.
I also added a test on `binary.md` that ensures all expressions include a file offset in their serialized format. This can be expanded to statements if offsets are added to the remaining statement nodes without offsets.
TEST=Added binary_md_git_test.dart to ensure all expressions contain offset data going forward.
Change-Id: Ieaf80545f388b209f76be44db86ef07e80c8ad66
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/331540
Reviewed-by: Jens Johansen <jensj@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Nate Biggs <natebiggs@google.com>
This adds the representation field of an extension type declaration
as an abstract getter in the ExtensionTypeDeclaraiton marking it as
a ProcedureStubKind.RepresentationField
These are never used as interface targets and are therefore skipped
in the type flow analysis.
TEST=existing
Change-Id: Ie645e63e0995a31895e985a2025dccb1476d16bf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/330782
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Ömer Ağacan <omersa@google.com>
Reviewed-by: Mayank Patke <fishythefish@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
This adds a list of `Procedure`s to `ExtensionTypeDeclaration`. This is
meant to model representation fields and combined member signatures
computed from inherited non-extension type members.
These are not meant to be handled by the backends. The combined
member signature can be the interface target of an `InstanceInvocation`
expression but will always have a `.memberSignatureOrigin` value from
one of the original class members.
TEST=existing
Change-Id: I87768ed75a3c7126b0a30f8ccf06e46678c56db6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/330301
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
This renames ExtensionType.typeErasure and adds it to DartType. This also fixes the extension type erasure for when extension types are used in the arguments of an extension type.
DartType.extensionTypeErasure can be used by backends to easily
access the type without extension types for any type.
TEST=pkg/kernel/test/extension_type_erasure_test.dart
Change-Id: Ia49d273ed85111e3ae822720860a3e0be5ea0252
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/329960
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Reviewed-by: Aske Simon Christensen <askesc@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Dart2JS uses the file offset on these nodes to create JS source maps. We want to maintain these file offsets across the serialization layer when we compile from dill files rather than directly from sources.
This last CL resolves all diffs observed in test files in pkg/compiler/test/inference/data.
TEST=Updating tests for all of these in fasta offsets tests.
Change-Id: If84c7542b61c8d8b894b202c68841ccfe91dafa5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/328080
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Nate Biggs <natebiggs@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Dart2JS uses the file offset on these nodes to create JS source maps. We want to maintain these file offsets across the serialization layer when we compile from dill files rather than directly from sources.
Dart2JS tests were serializing via `--test-mode` flag and thus had incorrect source locations for some stack traces.
TEST=Updating tests for all of these in fasta offsets tests.
Change-Id: I33862462fbff84d88f8c51bdeb1efc5771cfb8b2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/327160
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Nate Biggs <natebiggs@google.com>
This CL introduces StructuralParameter and StructuralParameterType
classes. They are intended to replace TypeParameter and
TypeParameterType respectively where those were used as type
parameters defined by FunctionTypes. Previously, type parameters of
FunctionTypes were represented by TypeParameter objects with the
‘parent’ field set to null. By introducing StructuralParameter and
StructuralParameterType this CL unambiguously separates the two
notions of type parameters.
TEST=existing
Change-Id: Ida3feb7ad96a7b2acef55840eacba9e36bf2a3e7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/312264
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Aske Simon Christensen <askesc@google.com>
Commit-Queue: Chloe Stefantsova <cstefantsova@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Mayank Patke <fishythefish@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
This splits visitors for (Tree)Visitor(1) into pure interfaces, mixins,
and a base class with the base implementation. This is a step towards
avoid having an accidental default implementation where a static error
would have been preferable.
This extract a ConstantReferenceVisitor(1) and its corresponding
DefaultMixin from the Visitor(1) interface.
TEST=existing
Change-Id: Ibc55bed9cff76581deaade91a10600c17fafc6dd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/325704
Reviewed-by: Mayank Patke <fishythefish@google.com>
Reviewed-by: Ömer Ağacan <omersa@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
This simplifies handle the constructor/member and its corresponding
tearoff as a pair. Also it prepare for supporting tearoff of
extension type constructors and methods with the same name.
TEST=existing
Change-Id: Iea6cbc0250c8df6bd0f825068c1f3e865d938427
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/324202
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
We are experiencing diffs in Dart2JS source maps in `--test-mode` which does a serialization round trip. JS that should map to default parameters is instead mapping to the surrounding `Constructor` member since the FieldInitializer's offset is getting dropped.
This is affecting any Dart2JS split action build since those all end of serialiaing and deserializing the kernel. Any other tools using this serialization would also be affected.
TEST=Existing/will add in follow up.
Bug: https://github.com/dart-lang/sdk/issues/53466
Change-Id: Ibeccf9153c78e6c358806c7ded4dbc2dea49d8e6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/325020
Commit-Queue: Nate Biggs <natebiggs@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
When the CFE compiles code for DDC or dart2wasm, it lowers late final
fields to synthetic getters. In order to ensure that accesses to these
fields can still be type promoted, a flag is added to the kernel
`Procedure` class, to indicate that a getter was produced by lowering
a late final field. When deciding whether a property access is
promotable, the CFE checks this flag; if it's set, it treats the
getter like a field.
Includes a language test validating the fix.
Fixes https://github.com/dart-lang/sdk/issues/53462.
TEST=tests/language/inference_update_2/late_field_test.dart
Change-Id: I98b81f04a3342d851060e5b2a7265323b93bb4e3
Bug: https://github.com/dart-lang/sdk/issues/53462
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/324767
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This adds a typed `TypeParameter.declaration` property to be used
instead of the `parent` property. The `declaration` property holds the
declaration (Class, Method, Extension, etc.) that introduced the
type parameter. `GenericFunction` is the subset of `GenericDeclaration`
that is defined through a `FunctionNode`.
TEST=existing
Change-Id: Ie89e7f5fa12a7966507a250cacc098eb0ce6b30b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/323160
Reviewed-by: Mayank Patke <fishythefish@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Aske Simon Christensen <askesc@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
This renames InlineClass to ExtensionTypeDeclaration, and InlineType
to ExtensionType. Members of extension type declarations are called
extension type members instead of extension type declaration members
for "brevity".
TEST=existing
Change-Id: I91ed62533ddd345644492f04dc3310d007460288
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/316780
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
This removes ExtensionType, ExtensionTypeShowHideClause and
CallSiteAccessKind from package:kernel which where only used by the now
removed 'extension-types' experiment.
A follow-up CL will rename InlineClass/InlineType to
ExtensionTypeDeclaration/ExtensionType to match the names of the
Extension Type feature currently being implemented.
TEST=existing
Change-Id: I58d2e8b0a92ac61329ee161cc6884a2c0e6f87ae
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/316420
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
This adds FileUriConstantExpression, a subclass of ConstantExpression, to support correct file offset of annotations for augmentations and patches.
The FileUriExpression is used to carry the file uri of the expression
before constant evaluation.
TEST=general/patch_annotations
Change-Id: I0dc8a0cb97dd530fd1960785d38c2d5e4883c3dd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/311660
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
There is an `includeSources` option today to skip writing both source bytes as well as the line starts. Dart2JS needs the line starts to generate source maps from its modified dill. But the actual bytes aren't used passed a certain point. This new option would allow us to exclude the information we don't need from the dill.
For large applications this can reduce the size of the dill by close to 50%.
Change-Id: I5e69370ee30b2fb856320c346b12662893cb595b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/293761
Reviewed-by: Jens Johansen <jensj@google.com>
This flag is set when a variable declaration is moved earlier in
order to be available for subsequent code that couldn't contain
its declaration. For instance variables declared in patterns which
needs to be declared before the matching expressions that initialize
them.
TEST=existing expectations tests
Change-Id: I80ab1138f08f725f3e01905c1a57a1483f809328
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/291820
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Jens Johansen