This project is read-only.

Bug in batching of graph mutation notifications?

Apr 23, 2013 at 2:39 PM
I think there is a flaw in the implementation of DoNotificationLayout() in GraphLayout.GraphElements.cs (see code quoted below). This method is called whenever a graph mutation has occurred. I believe the intention is that, if multiple graph mutations occur in rapid succession, then calling OnMutation() and ContinueLayout() to redo the layout is deferred until _notificationLayoutDelay (5ms) after the most recent mutation.

However, the synchronization on _notificationSyncRoot at the start of the method and around the whole of the DoWork delegate means that if a 2nd mutation occurs within 5ms of the previous one, DoNotificationLayout() will actually block the caller until the initial delay has elapsed, rather than returning immediately as I believe might be intended.

This bug isn't very noticeable. Mutations that occur within the same Windows message pump cycle will still be batched, because _worker.RunWorkerCompleted will not be invoked until the caller returns control to the message pump. But it does introduce a 5ms delay into code that changes the underlying graph and breaks the batching of quickfire updates in different message pump cycles.

A fix is to replace
Thread.Sleep(_notificationLayoutDelay);
with
Monitor.Wait(_notificationSyncRoot, _notificationLayoutDelay);
in the code below, which releases the lock during the Sleep.

Here is the current code:
private void DoNotificationLayout()
        {
            lock (_notificationSyncRoot)
            {
                _lastNotificationTimestamp = DateTime.Now;
            }
            if (_worker != null)
                return;

            _worker = new BackgroundWorker();
            _worker.DoWork += (s, e) =>
            {
                var w = (BackgroundWorker)s;
                lock (_notificationSyncRoot)
                {
                    while ((DateTime.Now - _lastNotificationTimestamp) < _notificationLayoutDelay)
                    {
                        Thread.Sleep(_notificationLayoutDelay);
                        if (w.CancellationPending)
                            break;
                    }
                }
            };
            _worker.RunWorkerCompleted += (s, e) =>
            {
                _worker = null;
                OnMutation();
                ContinueLayout();
                if (HighlightAlgorithm != null)
                    HighlightAlgorithm.ResetHighlight();
            };
            _worker.RunWorkerAsync();
        }