Commit graph

6 commits

Author SHA1 Message Date
Siva Annamalai 5cb3b37c74 Revert "Refactor _Future."
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>
2022-06-09 16:51:55 +00:00
Lasse R.H. Nielsen 69f32d6ad7 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>
2022-06-09 15:32:33 +00:00
Srujan Gaddam f5a98e7676 [ddc] Unify pkg:js types and allow subtyping between them
Removes the lazy loading of the underlying type in LazyJSTypes.
As such, this removes the need to keep AnonymousJSType and
LazyJSType separate, and is therefore refactored to
PackageJSType. Similarly, subtyping is fixed such that
PackageJSTypes are all subtypes of each other.

Change-Id: If489defdbeb5cb932db802a7d146ad2fc393b12c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/207982
Commit-Queue: Srujan Gaddam <srujzs@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
2021-07-30 00:33:54 +00:00
Nicholas Shahan e352744fa4 [ddc] Fix failing debugger tests
- Accept the existing output in the golden file as a new baseline.
- Copy the golden file for the null safe version of the test and
  enable running the full golden diff.

Fixes: https://github.com/dart-lang/sdk/issues/44025

Change-Id: Ie5b3e7c7689d17fca8a01e970b28fc815a6b9851
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/172688
Reviewed-by: Mark Zhou <markzipan@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
2020-11-20 23:14:33 +00:00
Vijay Menon f346cb7f77 Fix ddc debugger_test
This CL:
- Fixes https://github.com/dart-lang/sdk/issues/43987 (an exception in custom formatting code)
- Restores (most of) debugger_test.dart to passing (which tests above)
- Fixes a type caching error also exposed by the above test

Note, this skips the golden file comparison.  That appears to be very broken
(see comment).

Change-Id: I283b66a710f17765faed47aa099b0b8570e6cac1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/170022
Reviewed-by: Gary Roumanis <grouma@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Vijay Menon <vsm@google.com>
2020-11-02 23:31:30 +00:00
Joshua Litt d14ab0779a [dartdevc] Migrate tests/compiler/dartdevc_native to tests/dartdevc_2.
Change-Id: I9c486f29f3bcf8a6ecf481eb25ea61d0468049b2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/150667
Commit-Queue: Joshua Litt <joshualitt@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
2020-06-17 18:14:00 +00:00