When library_filters were given, we used to prefill the script_table_,
then just assume that any scripts not in the script_table_ must have
been filtered out. We wrote it this way to avoid checking the filters
in every GetScriptIndex call. But in some cases (eg mixins),
lib.LoadedScripts() can miss some scripts, so they'd be incorrectly
omitted from the table.
The new implementation lazy loads the scripts, the same way it works
when there are no library_filters. Skipped scripts are still placed in
the script_table_, but given an index of -1, and omitted from
script_table_entries_.
Bug: https://github.com/dart-lang/sdk/issues/49887
Change-Id: Ide938ddfa9a3750c72c615e296b1a23875e46ab8
TEST=CI (also manually tested that the bug is fixed, but it's a really fiddly setup and I'm not sure how to put it in a unit test. See bug for details)
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/260076
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Liam Appelbe <liama@google.com>
Currently the HeapSnapshotGraph represents the objects in the Dart VM
heap and how they relate to each other. They effectively form a directed
graph with a specific root node.
The API does allow traversing the graph by following an object's
outgoing references.
Though in certain situations it can be very useful to know the set of
objects referencing a given object (i.e. incoming references).
This CL adds such support, thereby allowing traversing edges in both
directions: Finding what an object references and who references an
object.
When loading a snapshot we therefore have to also compute the reverse
edges and store them. To offset any additional computation time and
memory consumption, we optimize the existing implementation by not using
growable List<int> objects for outgoing edges but rather typed-data
views into the existing Uint32List of edges.
Due to this optimization we make loading of snapshot faster and smaller,
in a simple benchmark we seem to get:
* JIT: 10% faster, 40% less RAM
* AOT: 30% faster, 40% less RAM
despite the additional compute & data structures of this API.
TEST=vm/dart{,_2}/heap_snapshot_referencees_test
Change-Id: Ief30bfb58c70364744eeb7f69967dd1f72ece807
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/260524
Reviewed-by: Tess Strickland <sstrickl@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
This will reduce the number of RPCs we need to do in package:coverage.
Benchmarked on a bunch of flutter test suites, and it halved the time
spent gathering coverage, bringing package:coverage's performance in
line with flutter's custom coverage collector. This unblocks migrating
flutter test to package:coverage.
Bug: https://github.com/flutter/flutter/issues/108313
Change-Id: I27651c7ce356d8b20c9c88444ad25d7677795a6d
TEST=Updated existing tests
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255720
Commit-Queue: Liam Appelbe <liama@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Closes https://github.com/dart-lang/sdk/pull/49478
TEST=Manual
GitOrigin-RevId: f4c9c6869dfe73639295e86574a021523b3d374d
Change-Id: I134a97caed4eec59d70e9cbca16b7e9a472cf2c1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/251902
Reviewed-by: Michael Thomsen <mit@google.com>
Commit-Queue: Alexander Thomas <athom@google.com>
Reviewed-by: Aske Simon Christensen <askesc@google.com>
Reviewed-by: Kevin Chisholm <kevinjchisholm@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
The required keyword means that adding a field to a class is a breaking
change, and needs a major version bump instead of a minor one. All the
fields are already nullable, and the constructors all use named
parameters, so removing this keyword is a minor (non-breaking) change.
Bug: https://github.com/dart-lang/coverage/pull/412
Bug: https://buganizer.corp.google.com/issues/236964692
Change-Id: I410d0f4359c003696570dfb11e3e2f7f179fb9ee
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/251443
Commit-Queue: Liam Appelbe <liama@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Remove them from tests.
(They should have been removed from tests before launcing 2.17.)
Change-Id: I546f6cb90fdf9e6ed1bb560f3715f9db163b7c68
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/250384
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Devon Carew <devoncarew@google.com>
Commit-Queue: Lasse Nielsen <lrn@google.com>
Fixes#49241
TEST=ci
Change-Id: I6117bf816fc8c4613cce66927f952fef75632725
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/248120
Reviewed-by: Alexander Thomas <athom@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Alexander Thomas <athom@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This option was enabled by default in https://dart-review.googlesource.com/c/sdk/+/149288
This change removes old logic behind --no-lazy-async-stacks
and makes --lazy-async-stacks/--no-lazy-async-stacks options no-op.
TEST=ci
Change-Id: I5726690e90e78dd2ac37d8c5944e388042fc3acf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/247780
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
This reverts commit 69f32d6ad7.
Reason for revert: We seem to have a number of tests failing with timeouts in CBUILD after this change, please see logs at
69f32d6ad7
Original change's description:
> Refactor `_Future`.
>
> This is a major rewrite of the `_Future` class,
> which is the default implementation of the `Future` interface.
>
> The main goal was to reduce the number of expensive type checks
> in the internal passing around of data.
> Expensive type checks are things like
> * `is _Future<T>` (more expensive than just `is _Future`, the latter
> can be a single class-ID check.
> * Covariant generic parameter checks (using `T` covariantly in a
> parameter forces a run-time type check).
>
> Also removed some plain unnecessary casts and turned some
> implicit casts from `dynamic` into `unsafeCast`s.
>
> This seems to be an success, at least on very primitive benchmarks, according to Golem:
> FutureCatchErrorTest 41.22% (1.9 noise)
> FutureValueTest 46.51% (2.8 noise)
> EmptyFutureTest 59.15% (3.1 noise)
> FutureWhenCompleteTest 51.10% (3.2 noise)
>
> A secondary goal was to clean up a very old and messy class,
> and make it clearer for other `dart:async` how to interact
> with the future.
>
> The change has a memory cost: The `_FutureListener<S,T>` class,
> which represents a `then`, `catchError` or `whenComplete`
> call on a `_Future`, now contains a reference to its source future,
> the one which provides the inputs to the callbacks,
> as well as the result future returned by the call.
> That's one extra memory slot per listener.
>
> In return, the `_FutureListener` now does not need to
> get its source future as an argument, which needs a covariant
> generic type check, and the methods of `_Future` can be written
> in a way which ignores the type parameters of both `_Future`
> and `_FutureListener`, which reduces complex type checks
> significantly.
>
> In general, typed code is in `_FutureListener`, which knows both
> the source and target types of the listener callbacks, and which
> contains the futures already at that type, so no extra type checking
> is needed.
> The `_Future` class is mostly untyped, except for its "public"
> API, called by other classes, which checks inputs,
> and code interacting with non-native futures.
> Invariants ensure that only correctly typed values
> are stored in the untyped shared `_resultOrListeners` field
> on `_Future`, as determined by its `_state` integer.
> (This was already partially true, and has simply been made
> more consistent.)
>
> Further, we now throw an error in a situation that was previously
> unhandled: When a `_Future` is completed with *itself*.
> That would ensure that the future would never complete
> (it waits for itself to complete before it can complete),
> and may potentially have caused weird loops in the representation.
> In practice, it probably never happens. Now it makes the error
> fail with an error.
> Currently a private `_FutureCyclicDependencyError` which presents
> as an `UnsupportedError`.
> That avoids code like
> ```dart
> import "dart:async";
> void main() {
> var c = Completer();
> c.complete(c.future); // bad.
> print("well!");
> var d = Completer();
> d.complete(c.future);
> print("shucks!");
> }
> ```
> from hanging the runtime by busily searching for the end of a cycle.
>
> See https://github.com/dart-lang/sdk/issues/48225
> Fixes#48225
>
> TEST= refactoring covered by existing tests, few new tests.
>
> Change-Id: Id9fc5af5fe011deb0af3e1e8a4ea3a91799f9da4
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/244241
> Reviewed-by: Martin Kustermann <kustermann@google.com>
> Commit-Queue: Lasse Nielsen <lrn@google.com>
TBR=lrn@google.com,kustermann@google.com,sra@google.com,sigmund@google.com,nshahan@google.com
Change-Id: I455be5a04b4c346df26d4ded0fa7388baccb0f8c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/247762
Reviewed-by: Siva Annamalai <asiva@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Alexander Aprelev <aam@google.com>
This is a major rewrite of the `_Future` class,
which is the default implementation of the `Future` interface.
The main goal was to reduce the number of expensive type checks
in the internal passing around of data.
Expensive type checks are things like
* `is _Future<T>` (more expensive than just `is _Future`, the latter
can be a single class-ID check.
* Covariant generic parameter checks (using `T` covariantly in a
parameter forces a run-time type check).
Also removed some plain unnecessary casts and turned some
implicit casts from `dynamic` into `unsafeCast`s.
This seems to be an success, at least on very primitive benchmarks, according to Golem:
FutureCatchErrorTest 41.22% (1.9 noise)
FutureValueTest 46.51% (2.8 noise)
EmptyFutureTest 59.15% (3.1 noise)
FutureWhenCompleteTest 51.10% (3.2 noise)
A secondary goal was to clean up a very old and messy class,
and make it clearer for other `dart:async` how to interact
with the future.
The change has a memory cost: The `_FutureListener<S,T>` class,
which represents a `then`, `catchError` or `whenComplete`
call on a `_Future`, now contains a reference to its source future,
the one which provides the inputs to the callbacks,
as well as the result future returned by the call.
That's one extra memory slot per listener.
In return, the `_FutureListener` now does not need to
get its source future as an argument, which needs a covariant
generic type check, and the methods of `_Future` can be written
in a way which ignores the type parameters of both `_Future`
and `_FutureListener`, which reduces complex type checks
significantly.
In general, typed code is in `_FutureListener`, which knows both
the source and target types of the listener callbacks, and which
contains the futures already at that type, so no extra type checking
is needed.
The `_Future` class is mostly untyped, except for its "public"
API, called by other classes, which checks inputs,
and code interacting with non-native futures.
Invariants ensure that only correctly typed values
are stored in the untyped shared `_resultOrListeners` field
on `_Future`, as determined by its `_state` integer.
(This was already partially true, and has simply been made
more consistent.)
Further, we now throw an error in a situation that was previously
unhandled: When a `_Future` is completed with *itself*.
That would ensure that the future would never complete
(it waits for itself to complete before it can complete),
and may potentially have caused weird loops in the representation.
In practice, it probably never happens. Now it makes the error
fail with an error.
Currently a private `_FutureCyclicDependencyError` which presents
as an `UnsupportedError`.
That avoids code like
```dart
import "dart:async";
void main() {
var c = Completer();
c.complete(c.future); // bad.
print("well!");
var d = Completer();
d.complete(c.future);
print("shucks!");
}
```
from hanging the runtime by busily searching for the end of a cycle.
See https://github.com/dart-lang/sdk/issues/48225Fixes#48225
TEST= refactoring covered by existing tests, few new tests.
Change-Id: Id9fc5af5fe011deb0af3e1e8a4ea3a91799f9da4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/244241
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Lasse Nielsen <lrn@google.com>
This is a reland of commit a091ff7b27
TEST=Check that a supplied URL conversion function is correctly applied when the `local` param is true.
Original change's description:
> [ Service / DDS ] Add method that can return local paths
>
> TEST=Check that a supplied URL conversion function is correctly applied when the `local` param is true.
>
> Change-Id: Ibe80b6229c574c976379a519baca5d1904b684b2
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/245040
> Reviewed-by: Ben Konyi <bkonyi@google.com>
> Reviewed-by: Kenzie Davisson <kenzieschmoll@google.com>
> Commit-Queue: Ben Konyi <bkonyi@google.com>
Change-Id: I87433a410715393f853a6538dbfe67391e0c773b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/246621
Commit-Queue: Helin Shiah <helinx@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
This reverts commit a091ff7b27.
Reason for revert: Breaks DevTools and DWDS on internal roll
Original change's description:
> [ Service / DDS ] Add method that can return local paths
>
> TEST=Check that a supplied URL conversion function is correctly applied when the `local` param is true.
>
> Change-Id: Ibe80b6229c574c976379a519baca5d1904b684b2
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/245040
> Reviewed-by: Ben Konyi <bkonyi@google.com>
> Reviewed-by: Kenzie Davisson <kenzieschmoll@google.com>
> Commit-Queue: Ben Konyi <bkonyi@google.com>
TBR=bkonyi@google.com,kenzieschmoll@google.com,helinx@google.com
Change-Id: I59d7687ba722fb1f5c706196691fc8471fe862ca
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/246057
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
TEST=Check that a supplied URL conversion function is correctly applied when the `local` param is true.
Change-Id: Ibe80b6229c574c976379a519baca5d1904b684b2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/245040
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Kenzie Davisson <kenzieschmoll@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Clears up confusion around function/field owner locations not
necessarily representing where the function/field was actually declared
(e.g., for functions and fields brought into a class via a mixin
application).
Fixes https://github.com/dart-lang/sdk/issues/45093
TEST=Documentation change
Change-Id: Ideaf17ec99d005459c60a2dd88f72b3485b32664
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/240481
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
This reverts commit ee5837aa7a.
Updated Intellij plugins have been published which support the new
message.
TEST=CQ
Change-Id: I81f6d878f036991a95cc1f58d3102015e77aa609
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/237741
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
These properties of `SocketStatistic` were mistakenly marked as
non-nullable but are not always returned as part of the response from
the service extension.
Change-Id: I996d8bdd9ff3b2acd00ce388582042b86fb95301
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/237632
Reviewed-by: Kenzie Davisson <kenzieschmoll@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>