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>
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>
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>
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 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>
Dart2JS and the VM prefer for Enum names to be stored inline in the Enum
object to avoid having the static array of all Enum values. A base class
for Enums is provided to avoid repeating the common fields and accessors
between Enum definitions.
Bug:
Change-Id: I783f863015b45f13317fda06e627d14844fe2ddf
Reviewed-on: https://dart-review.googlesource.com/16526
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Peter von der Ahé <ahe@google.com>
This flag indicates when a procedure's return type makes contravariant
use of a class type parameter. The front end will have to insert "as"
checks at any call sites that refer to such a procedure as their
interface target. Placing the flag on the procedure will allow the
front end to determine whether the check is needed once, at the same
time as it's determining the need for covariant parameter checks,
rather than when compiling individual call sites.
Once this lands I will follow up with CLs that cause the front end to
set the flag appropriately, and to generate the necessary "as" checks.
Change-Id: I989fc702bc233384eb5ea8cd630c8efd384ab248
Reviewed-on: https://dart-review.googlesource.com/14365
Reviewed-by: Samir Jindel <sjindel@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This CL adds internal binary format version to kernel binaries.
Kernel readers in the front-end and VM verify that the reader's format
version matches the format version in the binary file.
This improves detection and diagnostics of stale kernel binary files.
Change-Id: Ic69b16057397a84627040bdd6420ffa1852a4b3f
Reviewed-on: https://dart-review.googlesource.com/13280
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Reviewed-by: Kevin Millikin <kmillikin@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
This section contains a linear mapping between offsets of serialized nodes
and associated opaque binary metadata.
Note: this CL does not yet update C++ reader and only implements metadata in Kernel package.
C++ implementation will be updated if we agree that format seems flexible enough.
Bug:
Change-Id: Id433458afc6c2ea76c72f6a1901b9dc55b8f1696
Reviewed-on: https://dart-review.googlesource.com/9340
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
Before this CL we skipped procedure bodies in kernel_loader.cc by
parsing the body (but not storing anything).
With this CL we now skip them directly (i.e. don't read them at all)
in kernel_loader.cc by using the newly available extra indexes in kernel.
Change-Id: I48cf0599b2a85102c9008ff7c455785151ef3c9c
Reviewed-on: https://dart-review.googlesource.com/5764
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Samir Jindel <sjindel@google.com>
This adds more indexes to the kernel format so we know where classes
and procedures starts and stops. This allows for more random access.
E.g. one could now read the program index and jump directly to
library $i_1$, then read the library index and jump directly to class $i_2$,
read the class index and jump directly to procedure $i_3$.
The utilization (in this CL) is to not (always) read the procedure body
up front when loading kernel code on the dart side (ast_from_binary).
The observation is that - when running through the VM - almost none
of the bodies from the platform file are actually used.
This lowers the start-up cost which is noticeable for small programs
(e.g. hello world, or tests).
In this CL this is only done on the dart side and not on the C++ side,
that's for another CL.
Startup time:
dart2js: -1.84253% +/- 1.22157%
hello world: -11.1188% +/- 5.57892%
Running "time python tools/test.py -m release -cdartk language -j6":
real: -11.72% +/- 0.35%
user: -14.59% +/- 0.24%
sys: -12.71% +/- 0.61%
File size change (compiling with fasta to dill file incl. platform):
hello world: 0.88% (35,934 bytes).
dart2js: 0.97% (200,967 bytes).
Change-Id: I1f0ec121bc75bb17f11d3fade03da9815037d0bb
Reviewed-on: https://dart-review.googlesource.com/5262
Reviewed-by: Kevin Millikin <kmillikin@google.com>
This should take care of some or all flaky tests like:
TypePropagationTest_Kernel | test_forEach_async_inheritedStream
What was happening is that every tests adds /test.dart to AnalysisDriver,
which means that this file is scheduled for analysis at some point,
and then it also calls getResult() to get the resolved unit. When we
resolve the file for the first time, the ByteStore is empty, so we
build the corresponding Kernel file from scratch, and it has the offset.
But the second time we read the Kernel file from ByteStore. So, if we
manage to process the file as added first, and then as getResult(),
we fail because we cannot resolve the import directive. But if we
were not able to process the file as added, and just do getResult()
first (which also marks the file as added as ready), we succeed.
So, it was flaky.
R=ahe@google.com, kmillikin@google.com, paulberry@google.com, sigmund@google.com
Bug: https://github.com/dart-lang/sdk/issues/30863
Change-Id: I96151e3ebefcd212f2a7a1b2b22abb7d87ed4781
Reviewed-on: https://dart-review.googlesource.com/7782
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
This reverts commit 9a8621b60a.
Revert "Rework getElement() in resynthesizer."
This reverts commit e4fa080f69.
Revert "Create (empty) initializers for parameters resynthesized from Kernel."
This reverts commit 8df6c79b9d.
Revert "Run NonErrorResolverTest in strong mode. Extract not strong tests."
This reverts commit 9bdda4b1d3.
Revert "Move TypeProvider creation into KernelResynthesizer and create loadLibrary functions."
This reverts commit c59eaf1788.
Revert "Return SimpleIdentifier or PrefixedIdentifier from _buildIdentifier()."
This reverts commit 6d0515f9ca.
Change-Id: I1099ca715ce6287ab56808b7cc3abe0589e939c1
Reviewed-on: https://dart-review.googlesource.com/7550
Reviewed-by: Peter von der Ahé <ahe@google.com>
Before this CL to read the source (and line endings etc) for a specific
file index, one had to read at least part of the data for all previous
file indexes (e.g. read all the line endings).
This CL introduces an index to the included sources meaning that we have
random access based on file id, i.e. can go to the data concerning a
specific file id in constant time.
Benchmarks run with "time python tools/test.py -m release -cdartk language -j6"
shows that - of 5 runs - the runtime has changed as follows:
real: -3.93% +/- 1.03%
user: -3.41% +/- 0.54%
sys: No difference at 95%
(statistics by math stolen from ministat)
So it is ~4% faster to run the language tests (with above command),
shaving approximately 9 seconds off the real runtime.
Change-Id: I9e60a16958356b16b3da0bf6c01ffc5619deb976
Reviewed-on: https://dart-review.googlesource.com/3180
Reviewed-by: Samir Jindel <sjindel@google.com>
Alexander Aprelev