-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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
Optimize editor for input latency #161622
Comments
The core listener for text editors to react on content changes is:
These drive a ton of things on top such as:
I wonder how an async emitter would help here: yes, it would take away lag from the first character typing when the editor transitions into being dirty, but eventually we have to pay the price, so the lag would just happen later? Or is the idea to delay the event literally on idle time? |
@bpasero oh I missed the question there. The idea is to delay it until shortly after via setTimeout, so the text change should appear asap and the dirty indicator (as an example) would appear 1 or 2 frames later. ie: Current:
Desired:
There's some nuance here in what should be handled in the keypress task. For example currently the suggest widget is moved and updated in the keypress task. What we probably want is for the suggest widget to move in the keypress task but defer updating as it can be very expensive. Things like this we'll need to experiment with to see if splitting it up ends up with a worse UX and should be on the critical path. We may be able to optimize the list's |
Realized one of my laptops has a CPU similar to the average users (though a better GPU). Here's a screenshot of typing This is with a i7-8750H @ 2.2 GHz, didn't turn off turbo boost which can push it up to 4.10 GHz. Not entirely sure how that works but I think I can disable it in BIOS if needed. Though I haven't tested thoroughly on the laptop, I'm quite surprised that it seems to actually perform much worse than 4x CPU throttle on my primary machine (i7-12700KF @ 3.61 GHz, boost 5.00 GHz). I was expecting it to be the other way around. |
I drilled into a profile on my macbook to understand some parts a little more. Here are the details TLDR: An enormous amount of work seems to be spent just scheduling things, Latency
High level parts
Key press15.43ms (51% of critical path) Most expensive bottom up parts
What is it doing?Legend: 💡 We can probably improve or defer this fairly easily
|
The methodology, using the "Performance" tab was flawed, it exagerates timeouts (even when async stacks are disabled) and the overhead slows it by around 4x. A profile in the JavaScript Profiler tab is much more accurate and you can also perform the operation many times and then look as the slow parts in terms of the percentage of time they take up. By looking at the percentage instead of actual milliseconds it's much more reliable to get a good sample as it consolidates all calls. For example I recorded typing many bunch of characters, in top down you can now see a more accurate view of the function. This Now focusing on that function shows the breakdown of the call stack as a percentage of the total And 8.39% of the time after the model content changes: Looking into this some more now. |
Here's a breakdown of the CPU profile when typing a bunch of characters (~50, random), the suggest widget didn't show most of the time. This tree isn't complete, a lot is omitted here to remove noise vs just looking at the profile in devtools. The children of a node are not ordered and not necessarily directly below the parent or siblings of other children. ❌ = Too risky to touch This is a living document for now:
|
Before TextAreaHandler.onCursorStateChanged was taking approximately 4-16% of the total keypress task's runtime. After this is becomes 1-2ms. Part of #161622
Before TextAreaHandler.onCursorStateChanged was taking approximately 4-16% of the total keypress task's runtime. After this is becomes < 0.5ms. Part of #161622
Previously dealing with working copies was taking about 1.5% of the total runtime for the keypress function, this reduces it to around 0.2% by deferring all handling of the event until the text has rendered. Since the onDidContentChange event is not used, this should be a low risk change. Part of #161622
Profile when typing
|
This prevents double rendering, both in the keypress task and the following animation frame Part of #161622
After digging through how List.splice and in particular |
After pulling all the proposed changes into a single branch (tyriar/all_latency_changes) this is what pressing Current main: After the completion response comes back from the exthost this happens: Current main: I couldn't find many more wins here, most of it is bound by list splice and fetching the position of elements from the DOM (maybe something here could improve?). There are wins from moving to webgl as well but it would probably be more effort than it's worth to maintain. Here is a look at re-filtering the already visible suggest widget: Current main: |
Just discovered this issue: #27378 |
Using typometer, here are the results for xterm.js, with a few changes to get it closer to what we could expect as the upper bound of adopting the webgl renderer: Those are some pretty nice numbers 🙂, of course the model updates/events in bare xterm.js aren't nearly as costly as in vscode's editor though. The viewport is also quite small in xterm.js' demo which affects the time it takes to fill in the webgl buffers. Interestingly, those 2 ones that are very low with 8.2-8.3 averages are when I was taking a devtools performance profile. My theory is it's related to performance vs efficiency cores in my i12 CPU. They are also far more consistent during the performance profile: Compared to the other 3: Another thing I noticed are the big spikes that sometimes occur. This is actually why I did the performance profile to have a look at them. Here's an example (edited the screenshot as the iterations part was giant and couldn't be collapsed): I can't explain this as the work appears to be done right at the start, like all the other frames. Perhaps it's another application eating more CPU time? Or Chromium doing something and deciding to drop those frames? I tried to set process affinity/priority which didn't change the results. Also the processes are not marks as "efficiency mode" in task manager regardless of what task manager said, I think this doesn't map to efficiency cores though. |
|
@Tyriar I don't want to add noise to this issue, but I'm excited to see you working on this. I've kept an eye on latency and though I could give some comparisons for context: I found VSCode quite good — a lower min/max/mean that emacs & vim — at least on a high-spec Macbook Pro. Specifically, from a couple of years ago, typometer got 9.7/27.1/16.8 for min/max/mean; relative to 48/77/60 for vim and 29/48/38 for emacs. I just reran it a couple of times now and got higher max & means in VSCode 1.72.2: 9.7/66.2/23.1 & 7.4/49.6/21.9. So maybe something has got slightly worse, or maybe the setups a different. Min is still impressively low! To the extent we get a few ms saving, that's appreciated — I think I can notice a difference when there's more immediate feedback; even small amounts of latency or jitter feel worse (maybe needs a blind test though :) ) |
@max-sixty good to hear, I've been trying to squeeze out ms here and there for the past month or so, a lot of the changes are still in review though 🙂 |
For the unexplained xterm.js numbers, it appears to be related to extensions, incognito fixes the problem. Perhaps it waits on some extension APIs when not profiling? |
Looking at the spikes in the profiles again I think I can explain it now: There is a task that does These are for intersection observers which xterm.js does indeed use. https://developer.chrome.com/en/blog/new-in-devtools-92/#computed-intersections There are other places where I disabled the textarea syncing to avoid intersection needing to be recomputed and the results seem to be better. There are still the occasional spikes: Zooming into this one, it's happening because the It's not clear why, but if Chromium doesn't fire it there isn't much we can do about it. It does appear to be a longer interaction but I'm guessing that's because something's blocking so it delays processing the keyup event: I also tried a JavaScript profile and it didn't reveal anything for the spikes. |
Here are some results of measuring latency in VS Code and other similar projects: Some thoughts:
Hardware used:
|
* Batch/defer history service navigation events Part of #161622 * Pass store through Event.accumulate * Remove duplicate function * Action feedback * 💄 Co-authored-by: Benjamin Pasero <[email protected]> Co-authored-by: Benjamin Pasero <[email protected]>
I'm going to call this done for now, here are the outcomes:
|
I recently did an exploration into input latency and found it can get pretty bad on slower machines. A lot of the problem is related to how we use synchronous event emitters/listeners work, performing their work after keypress but before the animation frame.
My proposal to improve is:
Tentatively assigning to October
The text was updated successfully, but these errors were encountered: