-
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 all commits
104e63d
f085765
fd2d73e
a0ebfd8
6a7bbf2
b79e833
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. I would not make this public. |
||
{ | ||
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 | ||
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. same here - internal unless theres a good reason. |
||
{ | ||
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); | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -461,10 +461,17 @@ private void Browser_KeyDown(object sender, KeyEventArgs e) | |
|
||
var synteticEventData = new Dictionary<string, string> | ||
{ | ||
[Enum.GetName(typeof(ModifiersJS), e.KeyboardDevice.Modifiers)] = "true", | ||
["key"] = e.Key.ToString() | ||
}; | ||
|
||
foreach(ModifiersJS modifier in Enum.GetValues(typeof(ModifiersJS))) | ||
{ | ||
if (((int)e.KeyboardDevice.Modifiers & (int)modifier) != 0) | ||
{ | ||
synteticEventData[Enum.GetName(typeof(ModifiersJS), modifier)] = "true"; | ||
} | ||
} | ||
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. This is unrelated to the debouncing logic right? Would you elaborate? 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. Thank you, a good improvement for sure 👍🏻 |
||
|
||
_ = ExecuteScriptFunctionAsync(browser, "eventDispatcher", synteticEventData); | ||
} | ||
|
||
|
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?