New/Rebased #460: Improve Search Performance #559
Conversation
When the user types into the search box the previous search is cancelled. This improves the speed with which the results can be obtained.
|
@drognanar @trollixx This rebases #460, so that PR can be closed. |
|
@trollixx do I need to do anything else to get this merged? I really want to use Zeal, but the slowness kills me. We have a clear performance win with this PR. |
|
I guess @drognanar doesn't have enough time for this, or (most likely) got tired with my very slow response time, so thanks for picking this up. There were a few things that I didn't like (that's why the original PR wasn't merged in the first place), so now I have to go over this once again. I'll try to get this merged as soon as possible, but can't promise definite unfortunately. What I see immediately, is that half of commits here should be squashed with corresponding changes from the other half. It would be great if you could submit one PR per an actual change, so I could quickly review smaller chunks instead of going over unrelated code changes. This way multiple commits are fine, as GitHub can squash them automatically on merge. |
|
I'll squash commits 4, 7, 8, and 9. The others seem to contain the bulk of the work here. Why not comment on individual commits? You seem to have problems with the way github PRs work, but per-commit comments would work just as well as single-commit PRs. |
I actually do. I used to Gerrit's workflow, where one commit is one review request. It also allows anyone to update the same request. Then the final version gets cherry-picked on approval, while the whole history and attribution are still preserved in Gerrit. With GitHub it's much more complicated, because it allows to either merge all commits and get a usually useless merge-commit, or squash everything, which is not an option for situations of unrelated changes in a single PR. |
Add query cancellation at a docset level. This allows the query to be cancelled midway a large docset run. Add caching to docset results. This should limit the number of items to search through for longer queries.
|
@mrhota thanks for reviving this change. I really appreciate it @trollixx could you comment on what is it exactly that prevents this PR from getting merged in (besides having to rebase it yet again), and if there's anything I can actually help with? As much as I remember I have responded to all of your concerns before and did not get any additional comments under my PR. Perhaps, if you have any concerns, they could be addressed in a follow up PR instead? |
|
You are right, let's get this in. Doing a quick test myself right now. |
|
I have refactored out the search cancellation (c8464b6) and parallel search (40b8d4a) from #460. I am closing this, as @drognanar is currently rebasing #460 into the last commit. @mrhota Thanks for your work on rebasing and squashing, it was quite helpful for spotting a few issues among all the changes. |
I rebased #460 onto master.