Add first version of runAsync.

Add first version of runAsync.

Committed: https://code.google.com/p/dart/source/detail?r=19768

[floitsch, 2013-03-01 14:08:21]

The current name is acceptable to us, but if you find a better one, please let
us know.

Other question:
should it be acceptable to enqueue multiple immediate callbacks?
The current implementation just needs one.

https://codereview.chromium.org/12380060/diff/1/sdk/lib/async/event_loop.dart
File sdk/lib/async/event_loop.dart (right):

https://codereview.chromium.org/12380060/diff/1/sdk/lib/async/event_loop.dart...
sdk/lib/async/event_loop.dart:51: class _AsyncRun {
Without the class the VM won't find the external function.

[Lasse Reichstein Nielsen, 2013-03-04 09:23:24]

lgtm

https://codereview.chromium.org/12380060/diff/3001/sdk/lib/async/event_loop.dart
File sdk/lib/async/event_loop.dart (right):

https://codereview.chromium.org/12380060/diff/3001/sdk/lib/async/event_loop.d...
sdk/lib/async/event_loop.dart:19: _asyncCallbacks = <_AsyncCallback>[];
Or use a ListQueue for the callbacks queue. Then you can removeFirst while it's
not empty and don't worry if someone adds to it.

https://codereview.chromium.org/12380060/diff/3001/sdk/lib/async/event_loop.d...
sdk/lib/async/event_loop.dart:20: for (int i = 0; i < callbacks.length; i++) {
Should we avoid running too much in the same event, to avoid the browser
declaring our thread a "long running script"?
E.g., have a stopwatch, and re-schedule after some milliseconds?

https://codereview.chromium.org/12380060/diff/3001/sdk/lib/async/event_loop.d...
sdk/lib/async/event_loop.dart:25: } catch (e) {
If you keep using the same ListQueue, you don't have to worry about putting
stuff back in.

https://codereview.chromium.org/12380060/diff/3001/sdk/lib/async/event_loop.d...
sdk/lib/async/event_loop.dart:29: _asyncCallbacks = [];
Missing type parameter.

https://codereview.chromium.org/12380060/diff/3001/sdk/lib/async/event_loop.d...
sdk/lib/async/event_loop.dart:72: external static void _enqueueImmediate(void
callback());
Why this class? You should be able to patch top-level functions too.

https://codereview.chromium.org/12380060/diff/3001/tests/lib/async/run_async3...
File tests/lib/async/run_async3_test.dart (right):

https://codereview.chromium.org/12380060/diff/3001/tests/lib/async/run_async3...
tests/lib/async/run_async3_test.dart:24: }));
Consider adding a runAsync both before and after the Timer.run, to show that
it's not dependent on order.

[floitsch, 2013-03-04 17:26:04]

I will give it another day to allow the other reviewers to have a look too.

https://codereview.chromium.org/12380060/diff/3001/sdk/lib/async/event_loop.dart
File sdk/lib/async/event_loop.dart (right):

https://codereview.chromium.org/12380060/diff/3001/sdk/lib/async/event_loop.d...
sdk/lib/async/event_loop.dart:19: _asyncCallbacks = <_AsyncCallback>[];
On 2013/03/04 09:23:24, Lasse Reichstein Nielsen wrote:
> Or use a ListQueue for the callbacks queue. Then you can removeFirst while
it's
> not empty and don't worry if someone adds to it.

The async library doesn't include the collection library. I would like to keep
it that way.
The Queue also adds quite some code. Something I would like to avoid too.

https://codereview.chromium.org/12380060/diff/3001/sdk/lib/async/event_loop.d...
sdk/lib/async/event_loop.dart:20: for (int i = 0; i < callbacks.length; i++) {
On 2013/03/04 09:23:24, Lasse Reichstein Nielsen wrote:
> Should we avoid running too much in the same event, to avoid the browser
> declaring our thread a "long running script"?
> E.g., have a stopwatch, and re-schedule after some milliseconds?

No. runAsync guarantees to run before all events. If you starve your program
this way it's not worse than with while-loops.

https://codereview.chromium.org/12380060/diff/3001/sdk/lib/async/event_loop.d...
sdk/lib/async/event_loop.dart:25: } catch (e) {
On 2013/03/04 09:23:24, Lasse Reichstein Nielsen wrote:
> If you keep using the same ListQueue, you don't have to worry about putting
> stuff back in.

I considered ListQueues, but decided against them for the reason mentioned
above.

https://codereview.chromium.org/12380060/diff/3001/sdk/lib/async/event_loop.d...
sdk/lib/async/event_loop.dart:29: _asyncCallbacks = [];
On 2013/03/04 09:23:24, Lasse Reichstein Nielsen wrote:
> Missing type parameter.

Done.

https://codereview.chromium.org/12380060/diff/3001/sdk/lib/async/event_loop.d...
sdk/lib/async/event_loop.dart:72: external static void _enqueueImmediate(void
callback());
On 2013/03/04 09:23:24, Lasse Reichstein Nielsen wrote:
> Why this class? You should be able to patch top-level functions too.

I tried, but I couldn't get it to work.

https://codereview.chromium.org/12380060/diff/3001/tests/lib/async/run_async3...
File tests/lib/async/run_async3_test.dart (right):

https://codereview.chromium.org/12380060/diff/3001/tests/lib/async/run_async3...
tests/lib/async/run_async3_test.dart:24: }));
On 2013/03/04 09:23:24, Lasse Reichstein Nielsen wrote:
> Consider adding a runAsync both before and after the Timer.run, to show that
> it's not dependent on order.

Done.

[siva, 2013-03-07 02:39:16]

https://chromiumcodereview.appspot.com/12380060/diff/1011/runtime/lib/event_l...
File runtime/lib/event_loop_patch.dart (right):

https://chromiumcodereview.appspot.com/12380060/diff/1011/runtime/lib/event_l...
runtime/lib/event_loop_patch.dart:7: // TODO(floitsch): don't use the Timer to
enqueue the immediate callback.
Can we have an issue number associated with this TODO instead of a name.

https://chromiumcodereview.appspot.com/12380060/diff/1011/sdk/lib/_internal/c...
File sdk/lib/_internal/compiler/implementation/lib/async_patch.dart (right):

https://chromiumcodereview.appspot.com/12380060/diff/1011/sdk/lib/_internal/c...
sdk/lib/_internal/compiler/implementation/lib/async_patch.dart:26: //
TODO(floitsch): don't use the Timer to enqueue the immediate callback.
Ditto comment on TODO.

https://chromiumcodereview.appspot.com/12380060/diff/1011/sdk/lib/async/event...
File sdk/lib/async/event_loop.dart (right):

https://chromiumcodereview.appspot.com/12380060/diff/1011/sdk/lib/async/event...
sdk/lib/async/event_loop.dart:17: // Create a new list, to avoid that the we
grow too much if the callbacks
This comment does not read well.

https://chromiumcodereview.appspot.com/12380060/diff/1011/sdk/lib/async/event...
sdk/lib/async/event_loop.dart:27: _asyncCallbacks.getRange(i + 1,
_asyncCallbacks.length - i + 1);
shouldn't this be:
List remainingCallbacks = callbacks.getRange(i + 1, callbacks.length - i +1);

https://chromiumcodereview.appspot.com/12380060/diff/1011/sdk/lib/async/event...
sdk/lib/async/event_loop.dart:44: * Callbacks register through this function are
always executed in order and
registered

https://chromiumcodereview.appspot.com/12380060/diff/1011/tests/lib/async/run...
File tests/lib/async/run_async2_test.dart (right):

https://chromiumcodereview.appspot.com/12380060/diff/1011/tests/lib/async/run...
tests/lib/async/run_async2_test.dart:21: }
I think there should be a test where we add a bunch of callbacks and one of the
callbacks in the middle throws an exception. We then should check if the
remaining callbacks are executed.

[floitsch, 2013-03-08 20:04:40]

PTAL.

https://codereview.chromium.org/12380060/diff/1/sdk/lib/async/event_loop.dart
File sdk/lib/async/event_loop.dart (right):

https://codereview.chromium.org/12380060/diff/1/sdk/lib/async/event_loop.dart...
sdk/lib/async/event_loop.dart:51: class _AsyncRun {
On 2013/03/01 14:08:21, floitsch wrote:
> Without the class the VM won't find the external function.

Debugged with Ivan. For now the class is necessary.

https://codereview.chromium.org/12380060/diff/1011/runtime/lib/event_loop_pat...
File runtime/lib/event_loop_patch.dart (right):

https://codereview.chromium.org/12380060/diff/1011/runtime/lib/event_loop_pat...
runtime/lib/event_loop_patch.dart:7: // TODO(floitsch): don't use the Timer to
enqueue the immediate callback.
On 2013/03/07 02:39:17, siva wrote:
> Can we have an issue number associated with this TODO instead of a name.

Done.

https://codereview.chromium.org/12380060/diff/1011/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/lib/async_patch.dart (right):

https://codereview.chromium.org/12380060/diff/1011/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/lib/async_patch.dart:26: //
TODO(floitsch): don't use the Timer to enqueue the immediate callback.
On 2013/03/07 02:39:17, siva wrote:
> Ditto comment on TODO.

Done.

https://codereview.chromium.org/12380060/diff/1011/sdk/lib/async/event_loop.dart
File sdk/lib/async/event_loop.dart (right):

https://codereview.chromium.org/12380060/diff/1011/sdk/lib/async/event_loop.d...
sdk/lib/async/event_loop.dart:17: // Create a new list, to avoid that the we
grow too much if the callbacks
On 2013/03/07 02:39:17, siva wrote:
> This comment does not read well.

done.

https://codereview.chromium.org/12380060/diff/1011/sdk/lib/async/event_loop.d...
sdk/lib/async/event_loop.dart:27: _asyncCallbacks.getRange(i + 1,
_asyncCallbacks.length - i + 1);
On 2013/03/07 02:39:17, siva wrote:
> shouldn't this be:
> List remainingCallbacks = callbacks.getRange(i + 1, callbacks.length - i +1);

Done.

https://codereview.chromium.org/12380060/diff/1011/sdk/lib/async/event_loop.d...
sdk/lib/async/event_loop.dart:44: * Callbacks register through this function are
always executed in order and
On 2013/03/07 02:39:17, siva wrote:
> registered

Done.

https://codereview.chromium.org/12380060/diff/1011/tests/lib/async/run_async2...
File tests/lib/async/run_async2_test.dart (right):

https://codereview.chromium.org/12380060/diff/1011/tests/lib/async/run_async2...
tests/lib/async/run_async2_test.dart:21: }
On 2013/03/07 02:39:17, siva wrote:
> I think there should be a test where we add a bunch of callbacks and one of
the
> callbacks in the middle throws an exception. We then should check if the
> remaining callbacks are executed.

Done.

[siva, 2013-03-09 05:59:46]

lgtm

[floitsch, 2013-03-11 10:08:42]

Committed patchset #4 manually as r19768 (presubmit successful).