Reuse display_map snapshot where possible to reduce snapshot creation

[RemcoSmitsDev]

While debugging the slow performance of the project search, I discovered that in several cases we were creating multiple display_map snapshots within the same root-level function call. I noticed that creating a display_map snapshot is quite slow, and found that in some cases we were generating the snapshot multiple times. By reusing the snapshots, we see a clear performance improvement.

Note: Even though we've reduced the number of snapshots created, generating a display_map snapshot is still very slow and continues to appear prominently in my Instruments profile.

Release Notes:

• Improved project search performance by reducing the amount of display_map snapshot creations

[ConradIrwin]

Thanks for this!

I think the change is reasonable (in particular passing a display map instead of a cx to get selections makes sense). It obviously conflicts with a lot though, sorry for not reviewing faster.

That said, I'm curious to see some numbers. Do you have a reproduction case in search and how much faster does this make it? (I also wonder if there's more we can do to make creating snapshots faster... cc @maxbrunsfeld).

[RemcoSmitsDev]

Hey, thanks for the review and no worries!

I use the vscode-js-debug repo for project search testing because it gets some pretty large minified JavaScript files after a bun install & bun run compile. I use the search term ignore to get a decent amount of results.

What would be the best way to measure this? This PR should also improve the performance of some vim code paths that use selections, but that might be harder to measure, I guess.

Yeah, we might be able to improve the performance there, but hard to say because I'm not familiar with the code.

[ConradIrwin]

The way I've typically tested project search speed (though it isn't very scientific) is to set a global static when we start the search, and print out time elapsed when the 1st, 10th, 100th result has been added to the multibuffer.

For large minified files, I wonder if we aren't also running into similar issues as #16120...

[RemcoSmitsDev]

Yeah, I think we can tackle that later as Anthony is fixing the tabmap performance issue.

[RemcoSmitsDev]

@ConradIrwin So my measurement does not say it's faster, which makes sense in a way because I'm not hitting the code path that originally created 2 snapshots instead of one. This one happens when line_mode is enabled, in the case of getting the newest_adjusted selection, for example. But for multiple vim code paths, we now only create one display snapshot, and each snapshot creation takes about ~300ms. I'm not a vim person, so it's hard to measure what the exact performance gain is there, but should be a ~2x.

Before: (average 224.5260415ms) Search operation completed in 239.47475ms Search operation completed in 216.144541ms Search operation completed in 221.07775ms Search operation completed in 221.407125ms

After: (average 226.93803125ms) Search operation completed in 230.300875ms Search operation completed in 240.185833ms Search operation completed in 215.665708ms Search operation completed in 221.599709ms

[notpeter]

What's the status of this? (branch has gone stale and has merge conflicts) @RemcoSmitsDev are you waiting for a review from @ConradIrwin?

[RemcoSmitsDev]

if we want to keep go forward with this pr happy to fix the big merge conflicts. After that might need a review

[ConradIrwin]

I don't think this is going to be easy to land at all.

Closing for now, but we can come back to it later