dance icon indicating copy to clipboard operation
dance copied to clipboard

Keypresses race

Open CosineP opened this issue 1 year ago • 9 comments

Hi folks -

First of all, I want to thank you all for allowing me to use VSCode! I have long been hopelessly kakoune-pilled, and when I last tried VSCode there was no useful kakoune extension, so this is a huge win.

Here is the problem I am having: if my laptop gets below 20% (and tlp throttles my CPU), when I issue a bunch of commands they will occur in a fairly random order. The result is that I have to wait for the editor to respond after each keypress before I can press the next one if I want a consistent result.

Here's an easy way to reproduce: put your cursor at the beginning of a long line. Hold "e". If your cursor repeat is fast enough and your computer isn't a supercomputer, then your cursor jumps around to various ends of words, ending up at the end of a random word. (This happens to me even at full charge.)

If you allow me to bloviate for a moment, I can only assume this is because the handler for each keypress is issued asynchronously, but the effects are not await-ed before the next keypress is handled.

This is breaking for me when my laptop is under 20%, but over 20% usually the keypresses issue quickly enough to not matter. Current workaround is keep my laptop charged, because I really like this extension, but I still see races sometimes when I type really quickly. Fixing this would also allow dance to work on less powerful machines.

CosineP avatar Aug 12 '24 19:08 CosineP

I'm seeing the same, but on my desktop machine as well, so I would not say it's related to "less powerful" machines.

For me, a good reproducer is opening a file and keep pressing j. As soon as the cursor hits the bottom line of the viewport, scrolling slows down and the cursor jumps around in circles due to the race condition. It's quite annoying.

sgraf812 avatar Aug 25 '24 08:08 sgraf812

If I'm not mistaken, this code currently implements j:

https://github.com/71/dance/blob/7bf14636eac5422dd22af9964c2555ee8d3e4ef6/src/commands/select.ts#L318-L327

Perhaps it would be better to maintain a globally coherent array of selections that is asynchronously updated with diffs. I think that's what VSCodeVim does. At least they are using cursorMove commands to update the cursor position with a diff rather than with an updated position:

https://github.com/VSCodeVim/Vim/blob/33ac373dde7d14260d0ac1133bf1ba25a39dd556/src/actions/motion.ts#L85

sgraf812 avatar Aug 25 '24 09:08 sgraf812

I encounter the race condition most when switching between modes. Maybe implementing some kind of locking mechanism as soon as a mode change occurs would help? Then all keypresses that occur during the mode switch will wait until after the mode switch is complete.

It seems VSCodeVim also has this problem: https://github.com/VSCodeVim/Vim/issues/4283

JJK96 avatar Apr 18 '25 14:04 JJK96

I also encounter this occasionally, but am not sure how to fix it. The interface used to manipulate selections is purely synchronous, unlike the interface to manipulate edits which is asynchronous. Updating positions with diffs instead would likely require a significant rewrite of Dance (or perhaps we can manipulate selections synchronously, then compute a diff, then apply that diff asynchronously, but I'm not sure it will work).

We could try the "locking" solution, although it's not trivial (as Dance commands often spawn other commands, which with a trivial implementation would deadlock). For a command like j my first guess was that it wouldn't change anything, but thinking about it we have a couple of promises along the way, so it's possible we yield at some point and start processing another request in the meantime.

71 avatar Apr 19 '25 05:04 71

I've been investigating this lately, and really can't figure out where the problem comes from. I'm starting to doubt Dance is to blame, even though it only happens when Dance is enabled.

  • When timing Dance event handlers, none of them takes >10ms even if the editor is heavily slowed down (>1s to respond to clicks).
  • Disabling decorations doesn't fix it.
  • Memory seems to grow, but I don't know if it's because of Dance (I checked a few things that Dance allocates and keeps in memory, like recordings, but they grow too slowly to impact the overall memory usage, even after hours).

Now the reason why I don't think the problem comes from Dance (or rather, it's maybe a problem in VS Code that Dance surfaces): moving an heavily slowed down editor to another editor group makes it fast again, and moving it back to the previous editor group slows it down again. Edit: another strange behavior: if I scroll all the way to the top of a slow editor, things go fast again. If I scroll more than a dozen lines, it is slow again.

I don't think Dance ever behaves differently based on the editor group, so this is surprising. My guess at this point was that VS Code was caching decorations (which Dance heavily uses it), and moving the editor from one group to another fixed it, but disabling decorations didn't fix the problem.

I also disposed Dance (using the fix from #413, which disposes of more resources), and that didn't affect the slowness of the editor at all.

71 avatar Nov 08 '25 08:11 71

I think I've found the source of the issue. After setting editor.stickyScroll.enabled to false, all slow editors became responsive again. Re-enabling it with true let the editors remain responsive.

I'm guessing there is some cache somewhere with the sticky scroll which interacts badly with Dance (likely the "1 hidden selection above" message), and leaks memory over time.

The following may fix it (I'm currently testing it):

    "dance.modes": {
        "": {
            "hiddenSelectionsIndicatorsDecoration": {}
        }
    },

71 avatar Nov 13 '25 04:11 71

Thanks for investigating this! Let's hope this is indeed the cause!

JJK96 avatar Nov 13 '25 07:11 JJK96