Summary:
The constants table contains a concatenation of the variable-length encodings of
each constant used in the serialized component. Previously, we referenced the
constants in the code by index into the constants table, which requires reading
potentially the entire constants table to dereference. Now, we refer to the
constant by the binary offset into the constants table, which allows us to peek
into it in constant time.
Test Plan:
The constants table is used by all the precompiler modes, so the new encoding
will inherit the coverage of the existing encoding.
Results from debug and release precompiler bots are available under the
"cl-linux-try" button.
Change-Id: I4782bc0035d0b0cbd5bd6b9d086561ea601286c8
Reviewed-on: https://dart-review.googlesource.com/55470
Commit-Queue: Samir Jindel <sjindel@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
This CL updates the frontend_server to allow specifying what dill
file to initialize from.
Normally, it tries to initialize from the same dill file as is the
output. This is fine in many cases (e.g. flutter run always says it
should output to build/app.dill): The first time it compiles
everything, subsequent times (across restarts) it only recompiles
what's needed.
When running tests, however, the output is in a temporary directory,
so it doesn't work across restarts. The startup time for tests is
always rather high, because it always has to recompile everything.
This CL updates the frontend_server to allow specifying what dill
file to initialize from. Flutter can thus save the compiled test
file in a centralized location (e.g. build/test.dill) and load from
that across restarts.
Plumbing this into flutter yields the following result, when running
a single test (the one automatically created when creating a new
flutter project):
Before: ~5.5 seconds
Now, first run: ~5.5 second (unchanged)
Now, subsequent runs: ~3.4 seconds
Approximate breakdown:
Startup cost (e.g. if there's no test directory): ~ 800 ms
Starting up the actual frontend server: ~150 ms
Reading source from old dill, loading all relevant files
and invalidating source: ~ 300 ms
Compiling everything (or, rather, nothing as it's all
from the dill at this point): ~ 650 ms
Serializing: ~ 250 ms
Copying files: ~ 50 ms
Running the actual test: ~1100 ms
Total: ~3300 ms
`time flutter test --local-engine=host_debug --preview-dart-2`
says ~3.4 seconds...
To compare, --no-preview-dart-2 takes ~2.2 seconds,
so dart2 still has a (significantly) higher startup cost.
Moves the needle on https://github.com/flutter/flutter/issues/15767.
Change-Id: I36a2d05bc76e0193d369df42eb3b9f08416dd78f
Reviewed-on: https://dart-review.googlesource.com/51820
Reviewed-by: Kevin Millikin <kmillikin@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
Metadata is no longer written ahead of all nodes. Instead, metadata for
each node is written in the same context as the node itself (into a separate
buffer). This allows metadata to contain (serialize) arbitrary nodes
(for example, arbitrary DartTypes) and use serialization context of parent
nodes (such as declared type parameters).
However, with this change metadata looses the ability to reference
arbitrary AST nodes. This ability was overly restricted and had no
practical uses. (It was not possible to reference nodes which are not
reachable from root Component. As a consequence, it was not possible to
write references to arbitrary DartTypes.)
This change aligns the serialization capabilities of metadata with
how kernel AST nodes are serialized.
Change-Id: I027299a33b599b62572eccd4aa7083ad1dd2b3b3
Reviewed-on: https://dart-review.googlesource.com/54481
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Second try. First try was reverted because a previously empty URL (the
default 'always there' one) is now a null url instead, which wasn't
handled properly in the frontend_server.
See 47e9039512 for original details.
This reverts commit 4c9b712052.
Change-Id: I717eb927cbe29b6388e72f7e270428d234d44e4d
Reviewed-on: https://dart-review.googlesource.com/53580
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Peter von der Ahé <ahe@google.com>
This is the second try. First commit failed because another reading of
the binary was added after the change was originally made and before
actually landing it (and wasn't noticed when rebasing).
This reverts commit b298fc6d8f.
See 6e2536f585 for more information.
Change-Id: Ia0e7f8921de2cec8088654fa24950df13d846237
Reviewed-on: https://dart-review.googlesource.com/53560
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Change kernel serialization so that
a) We don't UTF8 encode strings up front, but only as needed
(if nothing else it makes it more obvious where the cost of stuff is)
b) Don't call toString on URIs to save them in a StringIndexer, only to
parse the string later to get a URI back again.
This shaves a bit of the serialization time.
Timings from running `flutter test` in flutter/packages/flutter:
Without change:
```
02:14 +2438 ~18: All tests passed!
02:12 +2438 ~18: All tests passed!
02:14 +2438 ~18: All tests passed!
```
with change:
```
02:10 +2438 ~18: All tests passed!
02:11 +2438 ~18: All tests passed!
02:09 +2438 ~18: All tests passed!
```
=> -2.5% +/- 1.83615%
Change-Id: I03e651bbef23445fb1901452a4efcc732789a168
Reviewed-on: https://dart-review.googlesource.com/51302
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Kevin Millikin <kmillikin@google.com>
Previously doubles was saved as strings in the string table,
with a DoubleLiteral holding a StringReference.
This can cause overhead in both computation time
(converting the double to and from string) as well as size
(e.g. a single usage of the previously unused double 1000000.42
would use (at least)
* 10 bytes for the characters
* 1 byte for the size
* 1 byte for the reference to the string
whereas saving it as a double would simply save the 8 bytes.
On the other hand the string table doesn't contain duplicates so
many usages of the same double will use more space.
The SDK dill file size decreases slightly (< 1 KB).
On a Dart file with 1M different doubles (0.42, 1.42, ..., 999999.42)
added to a list:
Before:
compile and write via fasta (non-strong-mode): 0:12.18
Reading (via dart, eager): 2500-2600 ms
Writing (to null sink) (after reading): 1600-1800 ms
Output dill file (via fasta compile): ~62 MB
After:
compile and write via fasta (non-strong-mode): 0:11.76
Reading (via dart, eager): 2050-2350 ms
Writing (to null sink) (after reading): 400-550 ms
Output dill file (via fasta compile): ~54 MB
Running the dill file is ~the same time, but "Maximum resident set size
(kbytes)" (from /usr/bin/time -v) decreases with ~4%.
On the other side, if it's 1M of the same doubles (0.42), while
compiling is ~the same speed, the output dill goes from 43MB to 50MB.
Surprisingly the "Maximum resident set size (kbytes)" still decreases
though (~3%).
Running flutter test in flutter/packages/flutter:
Before:
```
02:33 +2425 ~18: All tests passed!
02:28 +2425 ~18: All tests passed!
02:28 +2425 ~18: All tests passed!
```
After:
```
02:12 +2425 ~18: All tests passed!
02:11 +2425 ~18: All tests passed!
02:12 +2425 ~18: All tests passed!
```
So that's -12.0267% +/- 3.15253%
File size of a dill file of an arbitrary test using flutter is reduced by ~44 KB (~0.3%).
Change-Id: I64151376cde1dae6f0d02b3d96991bc432a994ae
Reviewed-on: https://dart-review.googlesource.com/41660
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Kevin Millikin <kmillikin@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Make all lists returned by ast_from_binary growable so the ast is more consistent with the ast generated by parsing dart code which generally contains growable lists as opposed to fixed-length lists.
This fixes a bug in the Flutter --track-widget-creation transform
where it would fail on an AST if it was deserialized from a binary file
rather than parsed from source code.
Change-Id: I4e6b199ed98df1b72d8d6844829341975a2c29c9
Reviewed-on: https://dart-review.googlesource.com/51780
Commit-Queue: Jacob Richman <jacobr@google.com>
Reviewed-by: Peter von der Ahé <ahe@google.com>
It complicates the intermediate language, none of the back ends are
using it, and it's not something that we want transformation writers
and code generators to deal with.
Change-Id: Ic79f7935dd8619bd233346bb25947e864f38a104
Reviewed-on: https://dart-review.googlesource.com/50440
Commit-Queue: Kevin Millikin <kmillikin@google.com>
Reviewed-by: Samir Jindel <sjindel@google.com>
Before, the part-URI was serialized as a file-URI and conflated with
source location information. The part-URI is the URI in the following
declaration in a Dart source file:
part "URI";
This is different from what we normally call a file-URI. A file URI
is used to point to the source location of the part declaration, not
the URI in the part declaration.
Furthermore, the field was serialized using writeUriReference which
only works for URIs that are in the uriToSource map on a Component.
Although this might seem like a safe optimization, it doesn't work
if the uriToSource map is omitted or if a part declaration refers to
a missing file.
Finally, due to the confusing use of fileUri, LibraryPart was
mistakenly implementing FileUriNode and annotations were stripped
of source locations if the source for the part were omitted from
uriToSource.
The partUri field is now an unresolved string that can be resolved
against either the parent library's import- or file-URI to obtain
either version as needed.
Change-Id: I255cb4eeaf89928292ab32a2f6be9ead6cc8cee1
Reviewed-on: https://dart-review.googlesource.com/49500
Commit-Queue: Peter von der Ahé <ahe@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Translate assert statements in async functions. The translation of
the condition subexpression can produce a sequence of statements that
have been hoisted out of it. These need to be guarded so they are
executed only when asserts are enabled.
For this purpose, we introduce an AssertBlock. The semantics of the
assert block:
assert { s0; ...; sn; }
is the same as:
if (assertsEnabled) { s0; ...; sn; }
where assertsEnabled is a more primitive nullary expression that is
true iff. assertions are enabled. We chose not to encode this using a
construct like assertsEnabled because (1) we would have to support it
appearing as an arbitrary expression which we don't currently need
and (2) it requires deeper pattern matching to detect and skip the
guarded code when desired.
The translation is more complicated because if there is a message
subexpression in an assert, it is conditionally evaluated only if the
assert's condition is false.
Fixes https://github.com/dart-lang/sdk/issues/28498
Change-Id: I0912a57104ede3160533e49f65b6fb79b76f1500
Reviewed-on: https://dart-review.googlesource.com/46442
Commit-Queue: Kevin Millikin <kmillikin@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
This CL changes:
* package:kernel/clone.dart: To not clone file offsets if the
file-uri we copy from is unknown.
* package:kernel/binary/ast_to_binary.dart: To not write out file
offsets if the file-uri is not written/indexed (which happens if
the file uri is not in the `Program.uriToSource` map).
* It reverts the file renaming part of 60a2cfa219 which
fixed the name collision problem only partially (it missed to
rename `runtime/lib/double.dart`, which is not possible because
there's already a `double_patch.dart` file).
Instead of renaming, we solve the problem by mangling the names as follows:
.../sdk/lib/core/double.dart -> dart:core/double.dart
.../runtime/lib/double.dart -> dart:core/runtime/lib/double.dart
.../runtime/lib/double_patch.dart -> dart:core/runtime/lib/double_patch.dart
* It changes the test to ignore any negative token positions. The vm
service does not just expose `-1` token positions for places where
there is no source, but also e.g. `-9` (which means TokenPosition::kMethodExtractor),
this was one reason why the appjit test failed.
* It changes the test to try to force-compile everything (unsure if that works).
Issue https://github.com/dart-lang/sdk/issues/32489
Change-Id: Ia9f42ca9d56d987e3041175add549bb7a133f269
Reviewed-on: https://dart-review.googlesource.com/46341
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
This CL adds two new kernel tools which can answer questions about
a dill file:
* count_breakdown.dart
Enumerates the different node types in the provided dill file and
counts them.
This has proved useful in the past to e.g. see how many doubles are
included in Flutter.
* size_breakdown.dart
Gives an overview of which parts of the dill file contributes how many
bytes.
This has proved useful in the past to see that the string table gets
huge when we have many doubles (because we currently save doubles in
kernel as strings).
It also adds error handling and usage information to existing tools.
Change-Id: Ie2ce9e4b8806d5975dba8f57445705a840be3474
Reviewed-on: https://dart-review.googlesource.com/42660
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Kevin Millikin <kmillikin@google.com>
This CL adds a test that bootstraps from a dill with a deferred library
in it. Before this CL it crashed, see bug #32245.
This CL "double fixes" the problem:
a) When lazy loading a dill file the _currentLibrary variable wasn't set
properly. This has been fixed.
b) When bootstrapping from a dill file, (almost) everything in the dill
will be included in the output so lazy loading it doesn't make sense.
Bug: 32245.
Change-Id: I94980608e19633fd31b522651a4a05b9cf7beabd
Reviewed-on: https://dart-review.googlesource.com/42600
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Kevin Millikin <kmillikin@google.com>
Before: Kernel static errors were represented by throwing a distinguished
error or else by calling a distinguished library const constructor.
Now: Kernel static errors are represented by InvalidExpression. To support
error reporting, InvalidExpression has an optional message and a file
offset. A back end can choose to signal these errors at any time; for
example, when deserializing the binary, or when compiling the procedure
containing the static error, or when the erroneous expression is evaluated
at run time.
InvalidStatement is removed because it can be encoded as ExpressionStatement
of InvalidExpression.
Future work:
* supporting static errors where an expression cannot appear in the AST
* allowing InvalidExpression to contain an Expression for error recovery
* adding a top-level list of static errors and warnings to the binary
Bug: https://github.com/dart-lang/sdk/issues/29840
Change-Id: Ifdfe9a76cee6cefed28061bf245be70531d2f413
Reviewed-on: https://dart-review.googlesource.com/31320
Commit-Queue: Kevin Millikin <kmillikin@google.com>
Reviewed-by: Dan Rubel <danrubel@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
When the body of a function node in a Procedure is lazy loaded the value
of the Procedures transformerFlags is not properly set before the body
has been read. That means that one can get the wrong result.
This CL makes sure to read the body (thereby setting the correct result)
before answering. The same thing is neccessary when setting the value to
avoid it being overwritten later.
Bug:
Change-Id: Id5e631e69885af0872b9644d02eb7b27dfb13391
Reviewed-on: https://dart-review.googlesource.com/21349
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Kevin Millikin <kmillikin@google.com>
We change the type of FileUriNode.fileUri from String to Uri, which in principle
doesn't change the binary format. However, we did notice that LibraryParts
weren't serialized as specified in binary.md, so we also fixed that.
Since fileUris are stored as strings in a separate table, Uri.parse is only called
once per unique URI.
Fasta only uses relativizeUri when printing diagnostics, and URIs stored in
expectation files (golden files) are relativized using String.replaceAll.
Change-Id: Ib2dc1b80c03a0cdaf84e48b8b3ba73b16bdf8a40
Reviewed-on: https://dart-review.googlesource.com/25421
Commit-Queue: Peter von der Ahé <ahe@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Assert initializers in Dart may be compiled directly to this form of initializer,
rather than through local initializers as is done now.
Bug:
Change-Id: Ia149ea3d1df5d1dc18be5636801604ffaf7ca7d8
Reviewed-on: https://dart-review.googlesource.com/14760
Commit-Queue: Samir Jindel <sjindel@google.com>
Reviewed-by: Kevin Millikin <kmillikin@google.com>
New type of AST nodes RedirectingFactoryConstructor is added to Kernel.
The goal is to keep the information about redirecting factory
constructors in the class for the linking phase.
Change-Id: I7477c617fa7fd4935ceb23098ec7c056d1f286ca
Reviewed-on: https://dart-review.googlesource.com/14740
Commit-Queue: Dmitry Stefantsov <dmitryas@google.com>
Reviewed-by: Peter von der Ahé <ahe@google.com>
This fixes some incorrect asserts that were breaking the debug bots.
The original revision is available in Patchset 1.
This reverts commit 26735519cb.
Bug:
Change-Id: Ifa599b7bff752dec4c505e10fd6db206e1abd977
Reviewed-on: https://dart-review.googlesource.com/23820
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Commit-Queue: Samir Jindel <sjindel@google.com>
This reverts commit f13f772bb2.
Reason for revert: dartk(p)-strong debug mode is completely broken
because we convert int values non-representable as Smi to Smi, which
triggers that assert. The code that does tagging checks that
Untag(Tag(x)) == x, however for our purposes this identity does not
need to hold because we only care about lower bits and ignore the sign.
Reverting to restore green-ness of DEBUG builds.
TBR=sjindel@google.com
Bug:
Change-Id: Id436cbe000d6dec8db3469070ed531327cc82d89
Reviewed-on: https://dart-review.googlesource.com/23661
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
The code for reading dill files via dart has a "debugPath" list that is
continously added and removed to in order to be able to debug what went
wrong if something goes wrong (e.g. if the dill file is invalid or the
reading code is wrong etc).
This CL wraps the updates of this list in assert so that we don't pay
for what we don't use in the general case. In a debug setting we can get
the functionality back via --checked.
On a benchmark of 10 runs, the time it takes to read vm_outline.dill 100
times after a 2 second warmup changes by -2.77% +/- 1.21%.
Bug:
Change-Id: I643b8dd778972621046fe76b536fd95e9bb66d1c
Reviewed-on: https://dart-review.googlesource.com/18820
Reviewed-by: Peter von der Ahé <ahe@google.com>
Reviewed-by: Dmitry Stefantsov <dmitryas@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
The introduced "constants" transformation can evaluate constant expressions. The
original use-sites of constant expressions are replaced by a new [ConstantExpression]
node, which points to a subclass of a new [Constant] class hierarchy. Constant
[Field]s and [VariableDeclarations]s will be removed, since all use-sites are
re-written.
The [Constant] class hierarchy is, similarly to the [DartType] class hierarchy, not
part of the AST tree (also has no parent pointer). The constants form a
DAG (directed acyclic graph).
There is no canonicalization requirement of the [Constant] objects referenced by the
AST (via [ConstantExpression]). Although it is beneficial to canonicalize them during
construction, since it reduces time spent in operator==/hashCode.
This CL furthermore adds support for a constant table in the binary format. Similarly
to [String]s, we canonicalize the constants before writing the table to the binary.
The constant table entries in the binary are written in a post-order way, to ensure
easy construction on the backend side.
The text format will be augmented with a "constants { ... }" section at the end,
which lists the constants in the same order as in the binary format.
The transformation can be used by those backends who choose to do so. It is not
enabled by default atm. It should therefore not affect analyzer, fasta or other
components.
Change-Id: I57cd9624fedcf537ab6870db76246149647bed21
Reviewed-on: https://dart-review.googlesource.com/14382
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Kevin Millikin <kmillikin@google.com>
The VM creates context objects even though it's only applicable in a
non-taken branch. This CL moves such a branch into its own function
and thus avoids the context object creation when loading a dill file
eagerly.
This CL further more reduces the number of context creations by not
lazy loading basically non-existing function node bodies (e.g. the body
of a FunctionNode in the outline).
The change has the following effect when loading platform/outline
100 times after 2 seconds warmup (statistics on 5 runs):
Outline, lazy loading disabled: -12.8133% +/- 2.26214%
Outline, lazy loading enabled: -40.9197% +/- 1.50042%
Platform, lazy loading disabled: No difference proven at 95.0% confidence
Platform, lazy loading enabled: -44.3347% +/- 0.613235%
Bug:
Change-Id: I9634e0a81f43efeb4e2524edb765d36072074f1e
Reviewed-on: https://dart-review.googlesource.com/18220
Reviewed-by: Peter von der Ahé <ahe@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
Loading vm_platform.dill 100 times after 2 seconds warmup:
Statistics on 5 runs:
It varies to much to tell on the runs I did.
Loading vm_outline.dill 100 times after 2 seconds warmup:
Statistics on 5 runs:
-7.84428% +/- 1.21335% at 95.0% confidence
Bug:
Change-Id: I34bbdca555ec50e1d16a5994c5cb46f845b4a4a3
Reviewed-on: https://dart-review.googlesource.com/18183
Reviewed-by: Dmitry Stefantsov <dmitryas@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
Changes in runtime:
Running python tools/test.py -m release -cdartk language -j6
Statistics on 3 runs:
real -1.21% +/- 1% at 95.0% confidence
user -2.01% +/- 0.29% at 95.0% confidence
sys No difference at 95%
Loading vm_platform.dill 100 times after 2 seconds warmup:
Statistics on 5 runs:
-15.3209% +/- 1.26028% at 95.0% confidence
Loading vm_outlin.dill 100 times after 2 seconds warmup:
Statistics on 5 runs:
-21.9672% +/- 0.48754% at 95.0% confidence
Bug:
Change-Id: I16474e32715df57922376d88baddd17a1cf73663
Reviewed-on: https://dart-review.googlesource.com/17788
Reviewed-by: Peter von der Ahé <ahe@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
Samir Jindel