Introduce optimized Timer.run queue for immediate runs.

Introduce optimized Timer.run queue for immediate runs.


BUG=

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

[Anders Johnsen, 2013-02-25 15:53:35]

Note that I changed the signature of Timer.run to _not_ return a new Timer
object. 'new Timer(0, ...)' can be used to really create a timer object, if it's
needed. As Timer.run 'Runs the given [callback] asynchronously as soon as
possible', a Timer object to control the callback seems a bit odd/overkill.

[Søren Gjesse, 2013-02-25 16:00:38]

lgtm

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

https://codereview.chromium.org/12316103/diff/1/sdk/lib/async/timer.dart#newc...
sdk/lib/async/timer.dart:57: static void run(void callback()) {
I don't see any problem with this not having any object associated for canceling
it. It is run "almost immediately" so the cancellation window is short and would
most likely have to use some knowledge about the order of execution if one
Timer.run function should be cancelled from another Tumer.run function.

https://codereview.chromium.org/12316103/diff/1/sdk/lib/async/timer.dart#newc...
sdk/lib/async/timer.dart:63: _queue = new Queue();
Please make a comment to why you create a new queue here (not starving real
timers I presume).

https://codereview.chromium.org/12316103/diff/1/tests/standalone/standalone.s...
File tests/standalone/standalone.status (right):

https://codereview.chromium.org/12316103/diff/1/tests/standalone/standalone.s...
tests/standalone/standalone.status:16: io/http_client_request_test: Pass, Fail #
Issue 8681
???

[floitsch, 2013-02-25 16:07:13]

LGTM.

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

https://codereview.chromium.org/12316103/diff/1/sdk/lib/async/timer.dart#newc...
sdk/lib/async/timer.dart:9: static Queue _queue = new Queue();
A list is good enough.

https://codereview.chromium.org/12316103/diff/1/sdk/lib/async/timer.dart#newc...
sdk/lib/async/timer.dart:54: * Returns a [Timer] that can be cancelled if the
callback is not necessary
Update comment.

https://codereview.chromium.org/12316103/diff/1/sdk/lib/async/timer.dart#newc...
sdk/lib/async/timer.dart:61: new Timer(const Duration(), () {
Duration(milliseconds: 0)

https://codereview.chromium.org/12316103/diff/1/sdk/lib/async/timer.dart#newc...
sdk/lib/async/timer.dart:62: var queue = _queue;
Use type.

https://codereview.chromium.org/12316103/diff/1/sdk/lib/async/timer.dart#newc...
sdk/lib/async/timer.dart:64: while (!queue.isEmpty) {
With the list:
function runQueued(List queued, int startAt) {
  for (int i = startAt; i < queued.length; i++) {
    Function f = queued[i];
    try {
      f();
    } catch(e) {
      _list.insertRange(0, 1);
      _list[0] = () => runQueued(queued, i + 1);
      throw;  // rethrow.
    }
  }
}
List queued = _list;
_list = new List<Function>();
runQueued(queued, 0);

[Anders Johnsen, 2013-02-25 16:25:18]

PTAL

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

https://codereview.chromium.org/12316103/diff/1/sdk/lib/async/timer.dart#newc...
sdk/lib/async/timer.dart:9: static Queue _queue = new Queue();
On 2013/02/25 16:07:13, floitsch wrote:
> A list is good enough.

Done.

https://codereview.chromium.org/12316103/diff/1/sdk/lib/async/timer.dart#newc...
sdk/lib/async/timer.dart:57: static void run(void callback()) {
On 2013/02/25 16:00:38, Søren Gjesse wrote:
> I don't see any problem with this not having any object associated for
canceling
> it. It is run "almost immediately" so the cancellation window is short and
would
> most likely have to use some knowledge about the order of execution if one
> Timer.run function should be cancelled from another Tumer.run function.

Done.

https://codereview.chromium.org/12316103/diff/1/sdk/lib/async/timer.dart#newc...
sdk/lib/async/timer.dart:61: new Timer(const Duration(), () {
On 2013/02/25 16:07:13, floitsch wrote:
> Duration(milliseconds: 0)

Done.

https://codereview.chromium.org/12316103/diff/1/sdk/lib/async/timer.dart#newc...
sdk/lib/async/timer.dart:62: var queue = _queue;
On 2013/02/25 16:07:13, floitsch wrote:
> Use type.

Done.

https://codereview.chromium.org/12316103/diff/1/sdk/lib/async/timer.dart#newc...
sdk/lib/async/timer.dart:63: _queue = new Queue();
On 2013/02/25 16:00:38, Søren Gjesse wrote:
> Please make a comment to why you create a new queue here (not starving real
> timers I presume).

Done.

https://codereview.chromium.org/12316103/diff/1/sdk/lib/async/timer.dart#newc...
sdk/lib/async/timer.dart:64: while (!queue.isEmpty) {
On 2013/02/25 16:07:13, floitsch wrote:
> With the list:
> function runQueued(List queued, int startAt) {
>   for (int i = startAt; i < queued.length; i++) {
>     Function f = queued[i];
>     try {
>       f();
>     } catch(e) {
>       _list.insertRange(0, 1);
>       _list[0] = () => runQueued(queued, i + 1);
>       throw;  // rethrow.
>     }
>   }
> }
> List queued = _list;
> _list = new List<Function>();
> runQueued(queued, 0);

Didn't do exactly this, but close.

https://codereview.chromium.org/12316103/diff/1/tests/standalone/standalone.s...
File tests/standalone/standalone.status (right):

https://codereview.chromium.org/12316103/diff/1/tests/standalone/standalone.s...
tests/standalone/standalone.status:16: io/http_client_request_test: Pass, Fail #
Issue 8681
On 2013/02/25 16:00:38, Søren Gjesse wrote:
> ???

Oops, reverting.

[Anders Johnsen, 2013-02-25 16:44:01]

PTAL :)

[floitsch, 2013-02-25 18:02:33]

LGTM.

https://codereview.chromium.org/12316103/diff/7001/sdk/lib/async/timer.dart
File sdk/lib/async/timer.dart (right):

https://codereview.chromium.org/12316103/diff/7001/sdk/lib/async/timer.dart#n...
sdk/lib/async/timer.dart:56: if (_runCallbacks.length == 1) {
Add comment explaining that this is an optimization that shouldn't be here.

https://codereview.chromium.org/12316103/diff/7001/sdk/lib/async/timer.dart#n...
sdk/lib/async/timer.dart:69: _runCallbacks.addAll(_runCallbacks.skip(i + 1));
runCallbacks

https://codereview.chromium.org/12316103/diff/7001/sdk/lib/async/timer.dart#n...
sdk/lib/async/timer.dart:70: _runCallbacks.addAll(runCallbacks);
newCallbacks

[Anders Johnsen, 2013-02-26 07:08:25]

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

[Anders Johnsen, 2013-06-11 12:14:31]

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

https://codereview.chromium.org/12316103/diff/1/sdk/lib/async/timer.dart#newc...
sdk/lib/async/timer.dart:54: * Returns a [Timer] that can be cancelled if the
callback is not necessary
On 2013/02/25 16:07:13, floitsch wrote:
> Update comment.

Done.

https://codereview.chromium.org/12316103/diff/7001/sdk/lib/async/timer.dart
File sdk/lib/async/timer.dart (right):

https://codereview.chromium.org/12316103/diff/7001/sdk/lib/async/timer.dart#n...
sdk/lib/async/timer.dart:56: if (_runCallbacks.length == 1) {
On 2013/02/25 18:02:33, floitsch wrote:
> Add comment explaining that this is an optimization that shouldn't be here.

Done.

https://codereview.chromium.org/12316103/diff/7001/sdk/lib/async/timer.dart#n...
sdk/lib/async/timer.dart:69: _runCallbacks.addAll(_runCallbacks.skip(i + 1));
On 2013/02/25 18:02:33, floitsch wrote:
> runCallbacks

Done.

https://codereview.chromium.org/12316103/diff/7001/sdk/lib/async/timer.dart#n...
sdk/lib/async/timer.dart:70: _runCallbacks.addAll(runCallbacks);
On 2013/02/25 18:02:33, floitsch wrote:
> newCallbacks

Done.