-
Notifications
You must be signed in to change notification settings - Fork 636
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DYN-8045: Fix hanged node search when the user types faster than search #15733
base: master
Are you sure you want to change the base?
Changes from 6 commits
104e63d
f085765
fd2d73e
a0ebfd8
6a7bbf2
b79e833
cde8b9d
e3dee54
66b749b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
using System; | ||
|
||
namespace Dynamo.Wpf.Utilities | ||
{ | ||
public static class JobDebouncer | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a class summary description. The PR as currently is already unifies both library (sidebar) and incontext node search into using the debouncer, but I can't find more instances of a debouncer being used in c# code (there are some in html/js webview code though).
chubakueno marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
public class DebounceQueueToken | ||
{ | ||
public long LastExecutionId = 0; | ||
public SerialQueue SerialQueue = new(); | ||
}; | ||
/// <summary> | ||
/// Action <paramref name="job"/> is guaranteed to run at most once for every call, and exactly once after the last call. | ||
/// Execution is sequential, and optional jobs that share a <see cref="DebounceQueueToken"/> with a newer optional job will be ignored. | ||
/// </summary> | ||
/// <param name="job"></param> | ||
/// <param name="token"></param> | ||
/// <returns></returns> | ||
public static void EnqueueOptionalJobAsync(Action job, DebounceQueueToken token) | ||
{ | ||
lock (token) | ||
{ | ||
token.LastExecutionId++; | ||
var myExecutionId = token.LastExecutionId; | ||
token.SerialQueue.DispatchAsync(() => | ||
{ | ||
if (myExecutionId < token.LastExecutionId) return; | ||
job(); | ||
}); | ||
} | ||
} | ||
public static void EnqueueMandatoryJobAsync(Action job, DebounceQueueToken token) | ||
{ | ||
lock (token) | ||
{ | ||
token.SerialQueue.DispatchAsync(() => | ||
{ | ||
job(); | ||
}); | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
//https://github.com/gentlee/SerialQueue/blob/master/SerialQueue/SerialQueue.cs | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @QilongTang do we need to update the about box for this? |
||
using System; | ||
using System.Threading; | ||
|
||
namespace Dynamo.Wpf.Utilities | ||
{ | ||
public class SerialQueue | ||
chubakueno marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
class LinkedListNode(Action action) | ||
{ | ||
public readonly Action Action = action; | ||
public LinkedListNode Next; | ||
} | ||
|
||
public event Action<Action, Exception> UnhandledException = delegate { }; | ||
|
||
private LinkedListNode _queueFirst; | ||
private LinkedListNode _queueLast; | ||
private bool _isRunning = false; | ||
|
||
public void DispatchAsync(Action action) | ||
{ | ||
var newNode = new LinkedListNode(action); | ||
|
||
lock (this) | ||
{ | ||
if (_queueFirst == null) | ||
{ | ||
_queueFirst = newNode; | ||
_queueLast = newNode; | ||
|
||
if (!_isRunning) | ||
{ | ||
_isRunning = true; | ||
ThreadPool.QueueUserWorkItem(Run); | ||
} | ||
} | ||
else | ||
{ | ||
_queueLast!.Next = newNode; | ||
_queueLast = newNode; | ||
} | ||
} | ||
} | ||
|
||
private void Run(object _) | ||
{ | ||
while (true) | ||
{ | ||
LinkedListNode firstNode; | ||
|
||
lock (this) | ||
{ | ||
if (_queueFirst == null) | ||
{ | ||
_isRunning = false; | ||
return; | ||
} | ||
firstNode = _queueFirst; | ||
_queueFirst = null; | ||
_queueLast = null; | ||
} | ||
|
||
while (firstNode != null) | ||
{ | ||
var action = firstNode.Action; | ||
firstNode = firstNode.Next; | ||
try | ||
{ | ||
action(); | ||
} | ||
catch (Exception error) | ||
{ | ||
UnhandledException.Invoke(action, error); | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this still required in addition to the search operations taking a lock out on the queue token?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lock is in case Search is used in parallel. As of now that could only happen if the user searches in the library and workspace simultaneously, since both have independent serial queues. We could mandate a unified search queue (for library, inCanvasSearch, and in the future) or have this lock to allow several search queues in parallel.