Performance improvements #51
Conversation
Moving up and down and making a selection do not change the list of matches, so we can eliminate needless computation from these actions with a safe way to pass along precomputed matches to a new Search instance. This makes a big difference in performance when the list of choices is large (> 1k).
Avoid unnecessary transcoding of inputs and map in one go rather than two.
- Don't re-score eliminated choices when appending to query - Cache previously calculated character indexes on choices
|
Breaking it down: cf01f26 Prefer readers to ivars The ivars are intentional: I try to stick to ivars from inside a class and methods from outside, so it's clear what's being referenced. I just removed one inconsistent use in 4f75d9c. c378d29 Optimize Search commands that don't affect matches I'd thought about this idea before but didn't implement it. I rarely use ^P and ^N, but your change is clearly a huge win even for me. There's no indication of this in the code, but Search is part of the functional core, which shouldn't do any mutation. I took your idea and implemented it without mutation in ad5bb6e. 08ff727 Improve startup time This is a pure win. It never even crossed my mind to check string validity first. f695810 Optimize matching This is another thing that I've considered doing. However, I have a long-lived branch (scoring_redesign) that it conflicts with. I've been struggling with this branch on and off because it's very slow in certain situations, and has some ugly code, but provides nicer matching in some situations. Specifically, it has special word boundary handling intended to achieve the goals of #24 and #30. I think that caching like you've introduced here should go in eventually, but I want to figure out the algorithm changes first (or abandon them, if that's the path I end up taking). These are nice patches; thanks. Two notes for the future: please match commit message style (50 char first line, lowercased; subsequent lines wrapped to 72 chars); and please make sure that tests pass (the last of your patches fails the tests, as noted by the Travis bot). I'm going to close this since three of the four changes are addressed. Can you rebase "Optimize matching" onto master, get the tests passing, and submit another PR with just that change? I want to close this issue, and having that new PR will ensure that I don't forget about it once the scoring stuff is settled. |
|
Thanks for the detailed feedback! Can't believe I forgot to run the test suite, that was a total bonehead move on my part. I'll rebase the "Optimize matching" commit and open another PR with that. |
|
Rebasing turned out to be a bit hairy and seemed unlikely to produce anything usable, so I opened an issue instead: #53 |
Hi @garybernhardt , I love selecta and have been using it a lot lately, and noticed a few areas where performance could be easily improved. These primarily help in repos with > 10k files (I have the misfortune of working in large Java repos), but the improvements are noticeable (to me) even on smaller repos. I estimate at least a 20% total speedup from these changes, more depending on how much one is navigating the file list instead of adding to the query.
That said, I understand that performance is not necessarily the primary goal of this project, so let me know if any of these optimizations feel too heavy-handed, and feel free to cherry-pick. (The search command optimization, for instance, seems like a pure win to me.)
Thanks again!