dart2js: Don't zone-register callbacks in async functions for every await.

dart2js: Don't zone-register callbacks in async functions for every await.

R=johnniwinther@google.com, lrn@google.com

Committed: https://github.com/dart-lang/sdk/commit/672df7c087e967183eefd2b942724a19a71012eb

[floitsch, 2015-08-11 15:26:35]

floitsch@google.com changed reviewers:
+ lrn@google.com

[floitsch, 2015-08-11 15:26:35]

This CL is based on https://chromiumcodereview.appspot.com/1163833002/.

Use the different patch-sets to see that the moved code almost didn't change.

[Lasse Reichstein Nielsen, 2015-08-12 09:46:55]

lgtm

https://codereview.chromium.org/1281523003/diff/60001/pkg/compiler/lib/src/el...
File pkg/compiler/lib/src/elements/modelx.dart (right):

https://codereview.chromium.org/1281523003/diff/60001/pkg/compiler/lib/src/el...
pkg/compiler/lib/src/elements/modelx.dart:1018: }
Unrelated change?

https://codereview.chromium.org/1281523003/diff/60001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/backend.dart (right):

https://codereview.chromium.org/1281523003/diff/60001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/backend.dart:673: Element
findAsyncAwaitHelper(String name) => find(asyncLibrary, name);
Seems like this just looks things up in the async library. 
It's currently only being used for async/await implementation, but the
implementation isn't bound to that.
Maybe rename to "findAsyncMember"?

https://codereview.chromium.org/1281523003/diff/60001/sdk/lib/_internal/js_ru...
File sdk/lib/_internal/js_runtime/lib/async_patch.dart (right):

https://codereview.chromium.org/1281523003/diff/60001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/async_patch.dart:9: ExceptionAndStackTrace,
We have AsyncError in dart:async that holds an error and a stack trace. Could
you just use that directly?

https://codereview.chromium.org/1281523003/diff/60001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/async_patch.dart:176: future._setValue(object);
You can throw away the errorCallback here since it's known to be a value. Might
save (slight amounts of) memory.

You can even just do:
  scheduleMicrotask(() { bodyFynctionOrErrorCode(async_error_codes.SUCCESS,
object); });

and avoid creating the new Future entirely.

https://codereview.chromium.org/1281523003/diff/60001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/async_patch.dart:329: }
Is this code identical to the one above in _asyncHelper? If so, abstract it!

https://codereview.chromium.org/1281523003/diff/60001/sdk/lib/_internal/js_ru...
File sdk/lib/_internal/js_runtime/lib/js_helper.dart (right):

https://codereview.chromium.org/1281523003/diff/60001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/js_helper.dart:2050: class
ExceptionAndStackTrace {
See dart:async's AsyncError. It has final members and a toString, but otherwise
it's the same.

[floitsch, 2015-08-12 13:30:46]

floitsch@google.com changed reviewers:
+ johnniwinther@google.com

[floitsch, 2015-08-12 13:30:47]

@Johnni: could you please look at the changes in modelx.

https://codereview.chromium.org/1281523003/diff/60001/pkg/compiler/lib/src/el...
File pkg/compiler/lib/src/elements/modelx.dart (right):

https://codereview.chromium.org/1281523003/diff/60001/pkg/compiler/lib/src/el...
pkg/compiler/lib/src/elements/modelx.dart:1018: }
On 2015/08/12 09:46:55, Lasse Reichstein Nielsen wrote:
> Unrelated change?

No. necessary, because we are looking up an element in a patch library.
Adding johnni as reviewer.

[floitsch, 2015-08-12 14:53:44]

https://codereview.chromium.org/1281523003/diff/60001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/backend.dart (right):

https://codereview.chromium.org/1281523003/diff/60001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/backend.dart:673: Element
findAsyncAwaitHelper(String name) => find(asyncLibrary, name);
On 2015/08/12 09:46:55, Lasse Reichstein Nielsen wrote:
> Seems like this just looks things up in the async library. 
> It's currently only being used for async/await implementation, but the
> implementation isn't bound to that.
> Maybe rename to "findAsyncMember"?

Done.

https://codereview.chromium.org/1281523003/diff/60001/sdk/lib/_internal/js_ru...
File sdk/lib/_internal/js_runtime/lib/async_patch.dart (right):

https://codereview.chromium.org/1281523003/diff/60001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/async_patch.dart:9: ExceptionAndStackTrace,
On 2015/08/12 09:46:55, Lasse Reichstein Nielsen wrote:
> We have AsyncError in dart:async that holds an error and a stack trace. Could
> you just use that directly?

not really. We want the unwrapException (of js_helper) to automatically extract
the error and the stacktrace.
Since AsyncError is a user-visible class, it can't do that for AsyncErrors.

https://codereview.chromium.org/1281523003/diff/60001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/async_patch.dart:176: future._setValue(object);
On 2015/08/12 09:46:55, Lasse Reichstein Nielsen wrote:
> You can throw away the errorCallback here since it's known to be a value.
Might
> save (slight amounts of) memory.
> 
> You can even just do:
>   scheduleMicrotask(() { bodyFynctionOrErrorCode(async_error_codes.SUCCESS,
> object); });
> 
> and avoid creating the new Future entirely.

I prefer to go through a future, but made error-handler `null`.

https://codereview.chromium.org/1281523003/diff/60001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/async_patch.dart:329: }
On 2015/08/12 09:46:55, Lasse Reichstein Nielsen wrote:
> Is this code identical to the one above in _asyncHelper? If so, abstract it!

Done.

https://codereview.chromium.org/1281523003/diff/60001/sdk/lib/_internal/js_ru...
File sdk/lib/_internal/js_runtime/lib/js_helper.dart (right):

https://codereview.chromium.org/1281523003/diff/60001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/js_helper.dart:2050: class
ExceptionAndStackTrace {
On 2015/08/12 09:46:55, Lasse Reichstein Nielsen wrote:
> See dart:async's AsyncError. It has final members and a toString, but
otherwise
> it's the same.

But it's publicly visible...

[Lasse Reichstein Nielsen, 2015-08-13 09:02:32]

https://codereview.chromium.org/1281523003/diff/60001/sdk/lib/_internal/js_ru...
File sdk/lib/_internal/js_runtime/lib/async_patch.dart (right):

https://codereview.chromium.org/1281523003/diff/60001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/async_patch.dart:9: ExceptionAndStackTrace,
Ah, so it needs to be distinguishable from any user-created value that can also
be thrown. That makes sense.

[floitsch, 2015-08-13 13:17:43]

@Johnni: I had to fix the Mockcompiler to include the private functions since
they are pulled in by the backend.

I couldn't just add them to the mock-library, since the source-mirror test would
otherwise fail (because it can't deal with private symbols).

[floitsch, 2015-08-17 12:43:31]

@Johnni: as discussed. changed the way how we find locals.

[Johnni Winther, 2015-08-17 13:10:19]

lgtm

[floitsch, 2015-08-17 13:57:39]

Committed patchset #10 (id:160001) manually as
672df7c087e967183eefd2b942724a19a71012eb (presubmit successful).