Simplify scheduleMicrotask.

Simplify scheduleMicrotask.

R=lrn@google.com

Committed: https://github.com/dart-lang/sdk/commit/88d064fe0120e49e0eb63807218ba2749a65a2d2

[floitsch, 2015-12-02 06:24:25]

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

[floitsch, 2015-12-02 06:24:25]

Turns out we never used the Zone field.

[Lasse Reichstein Nielsen, 2015-12-02 06:49:23]

lgtm

https://codereview.chromium.org/1487313002/diff/1/sdk/lib/async/schedule_micr...
File sdk/lib/async/schedule_microtask.dart (right):

https://codereview.chromium.org/1487313002/diff/1/sdk/lib/async/schedule_micr...
sdk/lib/async/schedule_microtask.dart:41: (entry.callback)();
Are you sure we shouldn't be setting the zone here and not binding the callback
instead. 
Seems overkill to create an extra closure for each microtask, and I think this
was my original plan.
On the other hand, the zone is currently just overhead, and if I decide to use
it, I can reintroduce it then.

https://codereview.chromium.org/1487313002/diff/1/sdk/lib/async/schedule_micr...
sdk/lib/async/schedule_microtask.dart:49: // good optimization.
Side-comment: This (still) really sucks!
If moving code to a separate function makes the code run faster, the compiler
should be doing it for you.

[floitsch, 2015-12-02 06:56:35]

https://codereview.chromium.org/1487313002/diff/1/sdk/lib/async/schedule_micr...
File sdk/lib/async/schedule_microtask.dart (right):

https://codereview.chromium.org/1487313002/diff/1/sdk/lib/async/schedule_micr...
sdk/lib/async/schedule_microtask.dart:41: (entry.callback)();
On 2015/12/02 06:49:22, Lasse Reichstein Nielsen wrote:
> Are you sure we shouldn't be setting the zone here and not binding the
callback
> instead. 
It is possible, but not trivial.
Currently, we call zone.bindCallback, if _rootScheduleMicrotask is run with a
Zone.current that is not the root-zone.
So, if we are in the root-zone, then nothing happens and we are fine anyway.
If we are in a zone, we need to pay attention that the zone doesn't override any
function that is invoked by `bindCallback`. That is feasible (since
ZoneSpecifications allow us to do that), but slightly more complicated than what
we have currently. I'm not sure it's worth it.


> Seems overkill to create an extra closure for each microtask, and I think this
> was my original plan.
> On the other hand, the zone is currently just overhead, and if I decide to use
> it, I can reintroduce it then.
I still hope that the root-zone is the more common case.

Also, I find this change convenient, since it allowed me to propose exposing the
_schedule functions below to the embedder (which would otherwise be much more
complicated).

Finally: I'm not 100% sure that we don't need to set the zone to the root-zone,
but in theory every zone is reset after a Zone.run. So that should be ok. But I
can set it explicitly to the Root-zone again.

https://codereview.chromium.org/1487313002/diff/1/sdk/lib/async/schedule_micr...
sdk/lib/async/schedule_microtask.dart:49: // good optimization.
On 2015/12/02 06:49:22, Lasse Reichstein Nielsen wrote:
> Side-comment: This (still) really sucks!
> If moving code to a separate function makes the code run faster, the compiler
> should be doing it for you.

I'm not even sure if it still is slower..

[floitsch, 2015-12-04 08:48:59]

Description was changed from

==========
Simplify scheduleMicrotask.
==========

to

==========
Simplify scheduleMicrotask.

R=lrn@google.com

Committed:
https://github.com/dart-lang/sdk/commit/88d064fe0120e49e0eb63807218ba2749a65a2d2
==========

[floitsch, 2015-12-04 08:49:00]

Committed patchset #1 (id:1) manually as
88d064fe0120e49e0eb63807218ba2749a65a2d2 (presubmit successful).