- Reduce the amount of messages being sent for Timer.run().

sdk/lib/io/timer_impl.dart

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 part of dart.io;
 
 // Timer heap implemented as a array-based binary heap[0].
 // This allows for O(1) `first`, O(log(n)) `remove`/`removeFirst` and O(log(n))
 // `add`.
 //
@@ skipping 114 matching lines @@
   static _Timer _lastZeroTimer;
 
   // We use an id to be able to sort timers with the same expiration time.
   // ids are recycled after ID_MASK enqueues or when the timer queue is empty.
   static int _ID_MASK = 0x1fffffff;
   static int _idCount = 0;
 
   static RawReceivePort _receivePort;
   static SendPort _sendPort;
   static int _scheduledWakeupTime;
+  // Keep track whether at least one message is pending in the event loop. This
+  // way we do not have to notify for every pending _firstZeroTimer.
+  static var _messagePending = false;
+
   static bool _handlingCallbacks = false;
 
   Function _callback;  // Closure to call when timer fires. null if canceled.
   int _wakeupTime;  // Expiration time.
   int _milliSeconds;  // Duration specified at creation.
   bool _repeating;  // Indicates periodic timers.
   var _indexOrNext;  // Index if part of the TimerHeap, link otherwise.
   int _id;  // Incrementing id to enable sorting of timers with same expiry.
 
   // Get the next available id. We accept collisions and reordering when the
@@ skipping 103 matching lines @@
 
 
   static void _notifyEventHandler() {
     if (_handlingCallbacks) {
       // While we are already handling callbacks we will not notify the event
       // handler. _handleTimeout will call _notifyEventHandler once all pending
       // timers are processed.
       return;
     }
 
-    if (_firstZeroTimer == null && _heap.isEmpty) {
+    if ((_firstZeroTimer == null) && _heap.isEmpty) {
       // No pending timers: Close the receive port and let the event handler
       // know.
       if (_receivePort != null) {
         _EventHandler._sendData(null, _sendPort, _NO_TIMER);
         _shutdownTimerHandler();
       }
     } else {
       if (_receivePort == null) {
         // Create a receive port and register a message handler for the timer
         // events.
         _createTimerHandler();
       }
       if (_firstZeroTimer != null) {
-        _sendPort.send(null);
+        if (!_messagePending) {
+          _sendPort.send(null);
+          _messagePending = true;  // Reset when the port receives a message.
+        }
       } else {
         var wakeupTime = _heap.first._wakeupTime;
         if ((_scheduledWakeupTime == null) ||
             (wakeupTime != _scheduledWakeupTime)) {
           _EventHandler._sendData(null, _sendPort, wakeupTime);
           _scheduledWakeupTime = wakeupTime;
         }
       }
     }
   }
 
   static void _handleTimeout(pendingImmediateCallback) {
-    int currentTime = new DateTime.now().millisecondsSinceEpoch;
+    // Fast exit if no timers have been scheduled.
+    if (_heap.isEmpty && (_firstZeroTimer == null)) {

[Søren Gjesse, 2015/01/20 15:36:27]

maybe assert(_receivePort == null)

[Ivan Posva, 2015/01/20 16:01:17]

Done.

+      return;
+    }
+
     // Collect all pending timers.
     var head = null;
     var tail = null;
-    // Keep track of the lowest wakeup times for both the list and heap. If
-    // the respective queue is empty move its time beyond the current time.
-    var heapTime = _heap.isEmpty ?
-        (currentTime + 1) : _heap.first._wakeupTime;
-    var listTime = (_firstZeroTimer == null) ?
-        (currentTime + 1) : _firstZeroTimer._wakeupTime;
+    if (_heap.isEmpty) {
+      // Only immediate timers are scheduled. Take over the whole list as is.
+      assert(_firstZeroTimer != null);
+      assert(_lastZeroTimer != null);
+      head = _firstZeroTimer;
+      tail = _lastZeroTimer;
+      _firstZeroTimer = null;
+      _lastZeroTimer = null;
+    } else {
+      assert(!_heap.isEmpty);

[zra, 2015/01/20 15:23:34]

Why is this assert needed?

[Ivan Posva, 2015/01/20 16:01:17]

It is more of a documentation and making sure the if-statement condition is not
changed.

+      // Keep track of the lowest wakeup times for both the list and heap. If
+      // the respective queue is empty move its time beyond the current time.
+      var currentTime = new DateTime.now().millisecondsSinceEpoch;
+      var heapTime = _heap.first._wakeupTime;
+      var listTime = (_firstZeroTimer == null) ?
+      (currentTime + 1) : _firstZeroTimer._wakeupTime;

[zra, 2015/01/20 15:23:34]

Indentation

[Ivan Posva, 2015/01/20 16:01:17]

Done.

 
-    while ((heapTime <= currentTime) || (listTime <= currentTime)) {
-      var timer;
-      // Consume the timers in order by removing from heap or list based on
-      // their wakeup time and update the queue's time.
-      assert((heapTime != listTime) ||
-             ((_heap.first != null) && (_firstZeroTimer != null)));
-      if ((heapTime < listTime) ||
-          ((heapTime == listTime) &&
-           (_heap.first._id < _firstZeroTimer._id))) {
-        timer = _heap.removeFirst();
-        heapTime = _heap.isEmpty ? (currentTime + 1) : _heap.first._wakeupTime;
-      } else {
-        timer = _firstZeroTimer;
-        assert(timer._milliSeconds == 0);
-        _firstZeroTimer = timer._indexOrNext;
-        if (_firstZeroTimer == null) {
-          _lastZeroTimer = null;
-          listTime = currentTime + 1;
+      while ((heapTime <= currentTime) || (listTime <= currentTime)) {
+        var timer;
+        // Consume the timers in order by removing from heap or list based on
+        // their wakeup time and update the queue's time.
+        assert((heapTime != listTime) ||
+               ((_heap.first != null) && (_firstZeroTimer != null)));
+        if ((heapTime < listTime) ||
+            ((heapTime == listTime) &&
+             (_heap.first._id < _firstZeroTimer._id))) {
+          timer = _heap.removeFirst();
+          heapTime = _heap.isEmpty ? (currentTime + 1) : _heap.first._wakeupTime;

[zra, 2015/01/20 15:23:34]

Long line

[Ivan Posva, 2015/01/20 16:01:17]

Done.

         } else {
-          // We want to drain all entries from the list as they should have
-          // been pending for 0 ms. To prevent issues with current time moving
-          // we ensure that the listTime does not go beyond current, unless the
-          // list is empty.
-          listTime = _firstZeroTimer._wakeupTime;
-          if (listTime > currentTime) {
-            listTime = currentTime;
+          timer = _firstZeroTimer;
+          assert(timer._milliSeconds == 0);
+          _firstZeroTimer = timer._indexOrNext;
+          if (_firstZeroTimer == null) {
+            _lastZeroTimer = null;
+            listTime = currentTime + 1;
+          } else {
+            // We want to drain all entries from the list as they should have
+            // been pending for 0 ms. To prevent issues with current time moving
+            // we ensure that the listTime does not go beyond current, unless
+            // the list is empty.
+            listTime = _firstZeroTimer._wakeupTime;
+            if (listTime > currentTime) {
+              listTime = currentTime;
+            }
           }
         }
-      }
 
-      // Append this timer to the pending timer list.
-      timer._indexOrNext = null;
-      if (head == null) {
-        assert(tail == null);
-        head = timer;
-        tail = timer;
-      } else {
-        tail._indexOrNext = timer;
-        tail = timer;
+        // Append this timer to the pending timer list.
+        timer._indexOrNext = null;
+        if (head == null) {
+          assert(tail == null);
+          head = timer;
+          tail = timer;
+        } else {
+          tail._indexOrNext = timer;
+          tail = timer;
+        }
       }
     }
 
     // No timers queued: Early exit.
     if (head == null) {
       return;
     }
 
     // If there are no pending timers currently reset the id space before we
     // have a chance to enqueue new timers.
@@ skipping 17 matching lines @@
         // one of the later timers which will set the callback to
         // null.
         if (timer._callback != null) {
           var callback = timer._callback;
           if (!timer._repeating) {
             // Mark timer as inactive.
             timer._callback = null;
           }
           callback(timer);
           // Re-insert repeating timer if not canceled.
           if (timer._repeating && (timer._callback != null)) {

[Søren Gjesse, 2015/01/20 15:36:26]

I know this is not part of the change, but reading the code in
_advandeWakeupTime it looks as if the new timeout of the repeating timer could
already be timed-out, and even timed out before the tail in the list being
processed.

Isn't that a problem?

E.g.

Repeating timer have a wakeupTime of 200 ms before current time when
_handleTImeout is called. It has an interval of 100 ms. When it has been called
the new wakeupTime is still before the time when_handleTimeout was called, and
later timers can be on the collected list.

[Ivan Posva, 2015/01/20 16:01:17]

I understand what you mean, but this is as if you have a repeating timer with a
0 repeat timeout. Newly scheduled timers only get a chance at the next run
through the event loop. There are no guarantees about ordering of timers, but we
do a best effort. There are similar issues about a repeating timer with a 10ms
that takes 15ms to process: You keep indefinitely falling behind. Not much we
can do about this at this point.

             timer._advanceWakeupTime();
             timer._addTimerToHeap();
           }
           // Execute pending micro tasks.
           pendingImmediateCallback();
         }
       }
     } finally {
       _handlingCallbacks = false;
       _notifyEventHandler();
     }
   }
 
   // Creates a receive port and registers an empty handler on that port. Just
   // the triggering of the event loop will ensure that timers are executed.
-  static _ignoreMessage(_) => null;
+  static _ignoreMessage(_) {
+    _messagePending = false;
+  }
 
   static void _createTimerHandler() {
     assert(_receivePort == null);
     _receivePort = new RawReceivePort(_ignoreMessage);
     _sendPort = _receivePort.sendPort;
     _scheduledWakeupTime = null;
+    _messagePending = false;
   }
 
   static void _shutdownTimerHandler() {
     _receivePort.close();
     _receivePort = null;
     _sendPort = null;
     _scheduledWakeupTime = null;
+    _messagePending = false;
   }
 
   // The Timer factory registered with the dart:async library by the embedder.
   static Timer _factory(int milliSeconds,
                         void callback(Timer timer),
                         bool repeating) {
     if (repeating) {
       return new _Timer.periodic(milliSeconds, callback);
     }
     return new _Timer(milliSeconds, callback);
   }
 }