Skip to content

Commit

Permalink
Revert "[vm] More efficient 'await' of not Future and completed _Future"
Browse files Browse the repository at this point in the history
This reverts commit 6b3d175.

Reason for revert: internal test failures b/235734143.

Original change's description:
> [vm] More efficient 'await' of not Future and completed _Future
>
> When awaiting a value which is not a Future or a completed
> built-in _Future, 'await' implementation can bypass heavyweight
> _Future/_FutureListener machinery and schedule micro-tasks directly.
>
> Benchmarks:
> JIT, x64:
> AsyncLiveVars.* +46-54% (bigger is better)
> Calls.AwaitAsyncCall -46% (less is better)
> Calls.AwaitAsyncCallInstanceTargetPolymorphic -46%
> Calls.AwaitAsyncCallClosureTargetPolymorphic -45%
> Calls.AwaitFutureOrCallInstanceTargetPolymorphicManyAwaits -45%
> Calls.AwaitFutureOrCall -60%
> Calls.AwaitFutureOrCallInstanceTargetPolymorphic -60%
> Calls.AwaitFutureOrCallClosureTargetPolymorphic -59%
>
> JIT, ia32:
> AsyncLiveVars.* +43-52% (bigger is better)
> Calls.AwaitAsyncCall -42% (less is better)
> Calls.AwaitAsyncCallInstanceTargetPolymorphic -42%
> Calls.AwaitAsyncCallClosureTargetPolymorphic -41%
> Calls.AwaitFutureOrCallInstanceTargetPolymorphicManyAwaits -39%
> Calls.AwaitFutureOrCall -55%
> Calls.AwaitFutureOrCallInstanceTargetPolymorphic -54%
> Calls.AwaitFutureOrCallClosureTargetPolymorphic -53%
>
> JIT, arm:
> AsyncLiveVars.* +64-71% (bigger is better)
> Calls.AwaitAsyncCallInstanceTargetPolymorphic -51% (less is better)
> Calls.AwaitAsyncCallClosureTargetPolymorphic -47%
> Calls.AwaitFutureOrCallInstanceTargetPolymorphicManyAwaits -48%
> Calls.AwaitFutureOrCall -64%
> Calls.AwaitFutureOrCallInstanceTargetPolymorphic -64%
> Calls.AwaitFutureOrCallClosureTargetPolymorphic -59%
>
> JIT, arm64:
> AsyncLiveVars.* +65-78% (bigger is better)
> Calls.AwaitAsyncCall -51% (less is better)
> Calls.AwaitAsyncCallInstanceTargetPolymorphic -51%
> Calls.AwaitAsyncCallClosureTargetPolymorphic -50%
> Calls.AwaitFutureOrCallInstanceTargetPolymorphicManyAwaits -49%
> Calls.AwaitFutureOrCall -69%
> Calls.AwaitFutureOrCallInstanceTargetPolymorphic -68%
> Calls.AwaitFutureOrCallClosureTargetPolymorphic -67%
>
> AOT, x64:
> AsyncLiveVars.* +55-61% (bigger is better)
> Calls.AwaitAsyncCall -47% (less is better)
> Calls.AwaitAsyncCallInstanceTargetPolymorphic -46%
> Calls.AwaitAsyncCallClosureTargetPolymorphic -47%
> Calls.AwaitFutureOrCallInstanceTargetPolymorphicManyAwaits -46%
> Calls.AwaitFutureOrCall -59%
> Calls.AwaitFutureOrCallInstanceTargetPolymorphic -59%
> Calls.AwaitFutureOrCallClosureTargetPolymorphic -58%
>
> AOT, arm:
> AsyncLiveVars.* 54-66% (bigger is better)
> Calls.AwaitAsyncCall -46-51% (less is better)
> Calls.AwaitAsyncCallInstanceTargetPolymorphic -46-50%
> Calls.AwaitAsyncCallClosureTargetPolymorphic -46-52%
> Calls.AwaitFutureOrCallInstanceTargetPolymorphicManyAwaits -45-50%
> Calls.AwaitFutureOrCall -63-68%
> Calls.AwaitFutureOrCallInstanceTargetPolymorphic -63-66%
> Calls.AwaitFutureOrCallClosureTargetPolymorphic -63-67%
>
> AOT, arm64:
> AsyncLiveVars.* +53-66% (bigger is better)
> Calls.AwaitAsyncCall -50-51% (less is better)
> Calls.AwaitAsyncCallInstanceTargetPolymorphic -50%
> Calls.AwaitAsyncCallClosureTargetPolymorphic -50-51%
> Calls.AwaitFutureOrCallInstanceTargetPolymorphicManyAwaits -49-50%
> Calls.AwaitFutureOrCall -66-68%
> Calls.AwaitFutureOrCallInstanceTargetPolymorphic -66-68%
> Calls.AwaitFutureOrCallClosureTargetPolymorphic -63-67%
>
> TEST=ci
> Issue: #48378
>
> Change-Id: I65e3702fcd816ee3fee876ff442b9887c035b1ec
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/243102
> Reviewed-by: Lasse Nielsen <lrn@google.com>
> Commit-Queue: Alexander Markov <alexmarkov@google.com>

# Not skipping CQ checks because original CL landed > 1 day ago.

Issue: #48378
Change-Id: I8ed0fca7234dd33f45997029b948a63341991ee1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/248353
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
  • Loading branch information
alexmarkov authored and Commit Bot committed Jun 14, 2022
1 parent 57eaad0 commit 0fd7f0c
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 60 deletions.
6 changes: 5 additions & 1 deletion pkg/vm_service/test/get_stack_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ final tests = <IsolateTest>[
(VmService service, IsolateRef isolateRef) async {
final result = await service.getStack(isolateRef.id!);

expect(result.frames, hasLength(6));
expect(result.frames, hasLength(10));
expect(result.asyncCausalFrames, hasLength(26));
expect(result.awaiterFrames, hasLength(13));

Expand All @@ -105,6 +105,10 @@ final tests = <IsolateTest>[
[equals('Regular'), anything], // Internal mech. ..
[equals('Regular'), anything],
[equals('Regular'), anything],
[equals('Regular'), anything],
[equals('Regular'), anything],
[equals('Regular'), anything],
[equals('Regular'), anything],
[equals('Regular'), endsWith(' _RawReceivePortImpl._handleMessage')],
]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ var tests = <IsolateTest>[
var frames = stack['frames'];
var asyncFrames = stack['asyncCausalFrames'];
var awaiterFrames = stack['awaiterFrames'];
expect(frames.length, greaterThanOrEqualTo(12));
expect(frames.length, greaterThanOrEqualTo(20));
expect(asyncFrames.length, greaterThan(frames.length));
expect(awaiterFrames.length, greaterThan(frames.length));
expect(stack['truncated'], false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ var tests = <IsolateTest>[
var frames = stack['frames'];
var asyncFrames = stack['asyncCausalFrames'];
var awaiterFrames = stack['awaiterFrames'];
expect(frames.length, greaterThanOrEqualTo(12));
expect(frames.length, greaterThanOrEqualTo(20));
expect(asyncFrames.length, greaterThan(frames.length));
expect(awaiterFrames.length, greaterThan(frames.length));
expect(stack['truncated'], false);
Expand Down
60 changes: 3 additions & 57 deletions sdk/lib/_internal/vm/lib/async_patch.dart
Original file line number Diff line number Diff line change
Expand Up @@ -372,69 +372,15 @@ class _SuspendState {
}
}

@pragma("vm:invisible")
@pragma("vm:prefer-inline")
void _awaitCompletedFuture(_Future future) {
assert(future._isComplete);
final zone = Zone._current;
if (future._hasError) {
@pragma("vm:invisible")
void run() {
final AsyncError asyncError =
unsafeCast<AsyncError>(future._resultOrListeners);
zone.runBinary(
unsafeCast<dynamic Function(Object, StackTrace)>(_errorCallback),
asyncError.error,
asyncError.stackTrace);
}

zone.scheduleMicrotask(run);
} else {
@pragma("vm:invisible")
void run() {
zone.runUnary(unsafeCast<dynamic Function(dynamic)>(_thenCallback),
future._resultOrListeners);
}

zone.scheduleMicrotask(run);
}
}

@pragma("vm:invisible")
@pragma("vm:prefer-inline")
void _awaitNotFuture(Object? object) {
final zone = Zone._current;
@pragma("vm:invisible")
void run() {
zone.runUnary(
unsafeCast<dynamic Function(dynamic)>(_thenCallback), object);
}

zone.scheduleMicrotask(run);
}

@pragma("vm:entry-point", "call")
@pragma("vm:invisible")
Object? _await(Object? object) {
if (_trace) print('_await (object=$object)');
if (_trace) print('_awaitAsync (object=$object)');
if (_thenCallback == null) {
_createAsyncCallbacks();
}
if (object is _Future) {
if (object._isComplete) {
_awaitCompletedFuture(object);
} else {
object._thenAwait<dynamic>(
unsafeCast<dynamic Function(dynamic)>(_thenCallback),
unsafeCast<dynamic Function(Object, StackTrace)>(_errorCallback));
}
} else if (object is! Future) {
_awaitNotFuture(object);
} else {
object.then(unsafeCast<dynamic Function(dynamic)>(_thenCallback),
onError:
unsafeCast<dynamic Function(Object, StackTrace)>(_errorCallback));
}
_awaitHelper(object, unsafeCast<dynamic Function(dynamic)>(_thenCallback),
unsafeCast<dynamic Function(Object, StackTrace)>(_errorCallback));
return _functionData;
}

Expand Down

0 comments on commit 0fd7f0c

Please sign in to comment.