We have two layers of laziness.
Lazy loading functions (the old one) disables itself when loading a
concatenated dill file.
Lazy loading class content (the new one) forgot to do that while not
being geared towards it either.
This means that it - when loading lazily - can look into the wrong
string table, the wrong link table etc.
This CL brings the laziness for classes on par with the laziness for
functions in that it is now disabled when loading a concatenated dill
file.
Change-Id: If3449d5aff34287221a14fb7e940779d93b1e7b3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/117145
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
This CL gives the option to load classes lazily (on by default),
meaning that fields, procedures, constructors and redirecting
factory constructors of classes are not initially read,
but instead read when (or if) needed.
The idea being that many classes aren’t needed, and that
spending time on reading them is thus just a waste of time.
This is especially true in modular compilation where a lot
of modules needs to be given as input as the compilation at
hand might technically rely on it, but in practice only very
few of the classes are actually needed (or very few classes
actually need their members).
The below numbers are comparing a VM without this change and without
the lazy class hierarchy member lists change with
b31566b297 (the base change for those
two changes).
For running a simple hello-world script, on my machine 500 times
and doing statistics on it I can measure the following differences:
With verbose, from kernel_service startup to compile complete:
Difference at 95.0% confidence
-17.8 +/- 0.555817
-24.3455% +/- 0.760206%
(Student's t, pooled s = 4.48379)
With verbose, total runtime measure by `time`
Difference at 95.0% confidence
-19.346 +/- 0.905217
-12.0333% +/- 0.563051%
(Student's t, pooled s = 7.30242)
Without verbose, total runtime measure by `time`
Difference at 95.0% confidence
-17.862 +/- 0.842905
-12.0635% +/- 0.569276%
(Student's t, pooled s = 6.79974)
For running a number of modular compilations I’ve recorded an
improvement in actual runtime in the order of 20-37% depending
on the circumstance. In a real build system, though, the
difference seems to be more in the order of around 5%.
Change-Id: Id329bcf2b01d12c12d7a49f2b8abacd9c2447f05
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/115703
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Add support for the `late` modifier on fields and local variables in
CFE, and `required` on named parameters When using the option
--enable-experiment=non-nullable the `isLate` and `isRequired` flags will be set
accordingly on Field and VariableDeclaration nodes.
Closes#37686Closes#37684
Change-Id: If37fc93defdabc5cc974f3f068f57752a665f4cb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/113036
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Reviewed-by: Dmitry Stefantsov <dmitryas@google.com>
Adds Extension node to kernel/ast, currently with serialization. This
is needed to support resolution of extension methods through dill
boundaries.
Adds (Source)ExtensionBuilder for build Extension nodes and
DeclarationBuilder as common superclass of ClassBuilder and
ExtensionBuilder.
Change-Id: Id945a40452f2729f9a4f390477dd6c76f9030eaa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/112256
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Dmitry Stefantsov <dmitryas@google.com>
A kernel library will say it has the newest language version if no
language version is set.
This basically preserves the current semantics of always being on the
newest version (if nothing is explicitly done), e.g. if creating a kernel
library by hand and not setting it.
The front_end also utilize this and just doesn't set it, if it isn't set.
Change-Id: I033fef13626ee034030ba8fc9d141b182ba1a5ba
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/111739
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
During constant evaluation, unused arguments to a const constructor are
thrown away after evaluation, since their values do not affect the
resulting instance constant. If such an unused argument ends up
unevaluated, any errors that would arise in the final evaluation are
not reported.
This CL adds space in the Kernel AST for saving these unevaluated
expressions so they can be checked during final constant evaluation.
Even though this is an incompatible change, no update is needed to the
VM code (except for the version bump), since the VM does not support
InstanceCreation nodes in the first place.
Change-Id: I4752562c1164efbba79eb018c15b07ed8354ce5f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105761
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Aske Simon Christensen <askesc@google.com>
Prior to this CL, we would issue a warning whenever a CanonicalNameError
was encountered.
This is in principal a good thing, but because we currently have no way
to detect if the sdk we get is the one we expect
(by any other measure than when it issues a CanonicalNameError) we often
issue these warnings for no "real reason" whenever, for instance,
the flutter sdk changes.
This CL splits the CanonicalNameError in two such that errors with
references to the sdk ("dart:" libraries) issue CanonicalNameSdkError
instead, an we then handle that differently. Namely we silently ignore
the error (i.e. don't issue a warning) and just don't initialize from
dill.
This should remedy the situation and be strictly better than to always
swallow CanonicalNameErrors.
Bug: 36032
Change-Id: Idbae0b5ee5b9843a5dbeb49b3c65ae25f5962e36
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105240
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Kevin Millikin <kmillikin@google.com>
TL;DR: Unbind canonical names doesn't do what you think it does and
probably shouldn't (ever) be used. This CL stops using it in a few places.
Longer version:
When loading a dill file it:
- First loads the table of canonical names. These have no references yet.
- When a canonical name is asked for its reference it creates one if it
doesn't yet have one.
- When loading, for instance, a library, it asks for the reference.
When unbinding a canonical name:
- It removes itself (the canonical name) from the reference
- It removes the reference in itself
- Note: Whatever has a pointer to the reference keeps it, and the
reference points to whatever node it already pointed to.
This also means, that if we have a dill file that's split in two and:
- Load #1
- Load #2
that works fine, but if we
- Load #1
- Unbind canonical names
- Binds canonical names
- Load #2
stuff is not bound correctly (and an error is thrown).
And - the cause of this bug:
- Load #1
- Load #2
everything is fine
- Unbind canonical names
- Binds canonical names
- Load #2'
stuff is not bound correctly --- references points to stuff loaded as #2,
not as #2'. On top of being weird, wrong and confusing it also caused wrong
things to be but into the class hierarchy which ultimatly caused the crash.
This CL fixes it by not calling unbind and force loading of dill files
(at specific call sites) to create new libraries
(and in the process overwriting references ".node").
Revert "[dartdevc] Retry ddc incremental compile on crash"
This reverts commit ecdbdf00b8.
Revert "[kernel_worker] retry on failure"
This reverts commit 43eebea5a3.
Fixes#36644
Bug: #36644
Change-Id: Id8f548179e6a409b01f2ebfa3219f94cb64b1c05
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/100380
Reviewed-by: Kevin Millikin <kmillikin@google.com>
Reviewed-by: Vijay Menon <vsm@google.com>
Reviewed-by: Jake Macdonald <jakemac@google.com>
Reviewed-by: Jenny Messerly <jmesserly@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
This for instance allows the VM to include all patches
(e.g. the previously missing `runtime/lib/date_patch.dart`)
in what's returned from `LoadedScripts()`.
Over time this also allows for `LoadedScripts()` to be simplified and
become faster (it's currently something like O(m + m*n) where m is the
number of results in DictionaryIterator(*this) and n is the number of
returned scripts).
Change-Id: I4b5a6a0fe666b774fc0987d099ed02e81ac97b43
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/98660
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Kevin Millikin <kmillikin@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
This is used in constant evaluation to represent const constructor
invocations with unevaluated field values or asserts.
Change-Id: I1d2d60a18f967a4dd195b3b5895db9a126c47803
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/98561
Reviewed-by: Kevin Millikin <kmillikin@google.com>
These arise when the constant evaluator partially evaluates collections
containing spreads or control-flow constructs with unevaluated
subexpressions. They are removed by the final constant evaluation.
Change-Id: Icdd155c4805cbcefe6aa4b45c2f85ec258e7bd36
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/95760
Commit-Queue: Aske Simon Christensen <askesc@google.com>
Reviewed-by: Dmitry Stefantsov <dmitryas@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Having a block as body instead of a list of expressions simplifies
code handling block expressions. Since it is always a block, it can
be left untagged in the serialized representation so the serialized
representation is not changed with this change.
Change-Id: I3c34f663041ff8cb4a530c2d6a148343e59c68fe
Reviewed-on: https://dart-review.googlesource.com/c/94223
Commit-Queue: Kevin Millikin <kmillikin@google.com>
Commit-Queue: Dmitry Stefantsov <dmitryas@google.com>
Auto-Submit: Kevin Millikin <kmillikin@google.com>
Reviewed-by: Dmitry Stefantsov <dmitryas@google.com>
Fasta can generate synthetic libraries.
Previously we didn't include them when serializing, but that leaves
references in the binary to libraries that doesn't exist which isn't
ideal.
This change includes them, and adds a flag to kernel Libraries such that
we know they are synthetic.
Change-Id: Ied25a21cd1f384d318347021bc7ec18dae3a4e05
Reviewed-on: https://dart-review.googlesource.com/c/91722
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Peter von der Ahé <ahe@google.com>
This achieves consistency with similar getters in the API.
This is technically a breaking change, since it changes a published
part of the Kernel API. Since the constants API is relatively new and
so far only used internally in the AOT compiler, the change is
expected to be unproblematic.
Closes https://github.com/dart-lang/sdk/issues/35696
Change-Id: I3ca30922580d226ccbdb6f77496983c21ef2102b
Reviewed-on: https://dart-review.googlesource.com/c/90220
Commit-Queue: Aske Simon Christensen <askesc@google.com>
Reviewed-by: Kevin Millikin <kmillikin@google.com>
Add Kernel syntactic support for constants constructed by
bool.fromEnvironment, int.fromEnvironment, and String.fromEnvironment.
These values are not necessarily available at compile time. Because
constants can depend on these values, there is also an unevaluated constant
that represents an expression depending on environment values.
This syntax is not yet produced by the Fasta compiler.
Change-Id: Ie96ea7f60a7efcd35ac802b320a00f398d41232e
Reviewed-on: https://dart-review.googlesource.com/c/88827
Commit-Queue: Kevin Millikin <kmillikin@google.com>
Reviewed-by: Aske Simon Christensen <askesc@google.com>
References can be null --- it's written as the byte [0].
Thus in the null case it would before be [0] and now be [0].
In the non-null case it would before be [1, xyz] (1 for Tag.Something
and xyz for the actual, now positive, uint30). Now it would be [xyz].
Change-Id: Ibc08d3afb7275b0429a4d6c5e667fbd381121489
Reviewed-on: https://dart-review.googlesource.com/c/85394
Reviewed-by: Kevin Millikin <kmillikin@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
Before this change we allowed to serialize some references as null,
and would only complain when trying to read the dill back in.
This CL changes that so it complains up front.
It is unknown if any of this would ever occur in practise.
Change-Id: Id5e1af89abfb88a2d4249bd439b53b062d7aeffa
Reviewed-on: https://dart-review.googlesource.com/c/85392
Reviewed-by: Kevin Millikin <kmillikin@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
Alignment of metadata sections in kernel binary files was introduced in
https://dart-review.googlesource.com/c/sdk/+/83543 in order to
keep bytecode instructions aligned without copying them from kernel
binaries.
When multiple kernel files are concatenated, metadata sections
could become unaligned if sizes of kernel files are not aligned.
This change adds padding into kernel binaries (before component index)
to make sure that size of kernel binaries is a multiple of 8.
Fixes https://github.com/dart-lang/sdk/issues/35443
Change-Id: Ic7dc0da66c699ef406a325e69fe277e9d9e3b836
Reviewed-on: https://dart-review.googlesource.com/c/87741
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Auto-Submit: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
This is a major revamp of bytecode metadata format. Now bytecode has
its own serialization mechanisms.
This CL adds 'bytecode component' metadata, which contains bytecode
object table and string table. All references from bytecode (constant
pools) to libraries, classes, members, types and strings now have a new
format. References to frequently used objects are represented as indices
in object table, while rarely used objects are written inline.
This allows VM to cache frequently used objects while reading bytecode.
Representation of strings is aligned with VM - string characters are
stored in separate pools of one-byte and two-byte strings. This allows
VM to avoid UTF-8 decoding and extra copying.
Closure declarations are now explicit. Type parameters no longer require
enslosing scopes when reading/writing them.
Benchmarks:
GenKernelKernelReadAllBytecode (Intel Core i5) +29.84%
GenKernelKernelReadAllBytecode (Intel Xeon) +28.74%
Change-Id: I4b80009733a8f8c038264af74f97c4e094b9e311
Reviewed-on: https://dart-review.googlesource.com/c/85469
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Régis Crelier <regis@google.com>
Reviewed-by: Zach Anderson <zra@google.com>
Previously we had several places where different uriToSource was used.
This was both weird and led to errors when something refering to
some state was used to look up in another state.
This CL makes it so we only have one source of truth.
To not include sdk sources when serializing a component that only mixes
something from the sdk in, (or extends it or...), the serialization is
changed slightly to keep track of which uris come from actual
implementation. Before the sdk sources was explicitly removed in the
incremental compiler, but we want to limit those kinds of things,
which is why we're doing it differently here.
Fixes#35215.
Bug: 35215
Change-Id: Iaa5618fcb0ea42b13aba7720f34a87a85144e047
Reviewed-on: https://dart-review.googlesource.com/c/85175
Reviewed-by: Peter von der Ahé <ahe@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
For all other "debugPath.add"s we've wrapped it in assert to avoid
doing it in the general case.
One apparently have slipped through.
Change-Id: Ib0dfc4a671428993e393d57a75db6612785e37b4
Reviewed-on: https://dart-review.googlesource.com/c/85161
Reviewed-by: Dmitry Stefantsov <dmitryas@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
Kallen Tu