Repository navigation
Conversation
- move the lookup update to a RequestProcessor task on the ExplorerManager selection processor, so the diffing no longer runs on the EDT - reschedule the task on every selection change so that rapid changes coalesce; the update reads the current selection when it runs, so a superseded update cannot leave stale state - UPDATE_DELAY (30ms) additionally debounces bursts; value derived from measured arrow-key repeat intervals Assisted-by: qwen3.8-27b
|
ready for review but i would like to add a cleanup commit later for this class + maybe reduce the javadoc text further since it repeats itself. |
|
i feared test failures since tests might expect the lookup to change right after selection updates |
I suspect more than just test code will expect that. This feels like something that should coalesce at the UI level somewhere before the EM is updated? |
|
yes that was supposed to be the second PR since I found some potential issues in event consumers (taking a look at this now). If we can't delay the lookup update itself, fixing all usages is the only remaining option - assuming its at all possible. But as soon any of the consumers is misbehaving - as demonstrated with the logger which can't log fast enough - the result is a full event queue and frozen NB. |
jtulach
left a comment
There was a problem hiding this comment.
- I am having problems with too slow arrow down/up too
- usually when something "strange" is in my clipboard
- I haven't seen this particular problem recently however
- moving the update to asynchonous task is (probably) correct
- just you need to wait for the last scheduled task when somebody queries the state
| * the selection when it runs, so a superseded update can never leave a stale | ||
| * state behind. | ||
| */ | ||
| private void scheduleUpdate() { |
There was a problem hiding this comment.
- As the failing tests indicate
- just the
schedule(later)fix can hardly be compatible with current behavior - the tests perform an action and expect to see the result immediately
- just the
- probably one would have to:
- immediately invalidate the state
- schedule the update in a background sliding task (as a95adf8 does)
- whenever someone queries the invalidated, not yet computed state ...
- ...await the computation with
task.waitFinished()
- something like that has been attempted in
ExplorerActionsImpl.ActionsStateUpdater - but yeah, when I see this code over all the years, I am wondering why we haven't done a better job...
There was a problem hiding this comment.
just the schedule(later) fix can hardly be compatible with current behavior
the tests perform an action and expect to see the result immediately
yes that is what I said and I agree. This won't work unfortunately. And the very exception which occurred indicates that something very similar had been tried before and reverted. (DelayedLkp)
we are stuck running this on EDT. What remains is to make consumers non-blocking.
|
closed in favor of "downstream" optimizations: |
while testing #9770 i noticed that I could easily lock up the IDE by selecting something in the tree and holding down the arrow key (it sounds like a Kraftwerk song!). Initially i thought it was a deadlock.. but the IDE recovered after a few minutes.
This was esp visible in dev builds since they log more due to assertions. Log showed:
the cause for the lockup was that the log thread couldn't deal with the back pressure of the assertion log exceptions happening during dev builds. However... this does also indicate that this shouldn't run on EDT in the first place. (so its good that it happened)
-> This moves the selection-triggered lookup update off the EDT + debounces event bursts.
commit:
RequestProcessortask on theExplorerManagerselection processor, so the diffing no longer runs on the EDTUPDATE_DELAY(30ms) additionally debounces bursts; value derived from measured arrow-key repeat intervalsAssisted-by: qwen3.8-27b
there is a second performance issue. But thats for another PR.