Skip to content
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

Remove search debounce (2x+ speedup) #2412

Closed

Conversation

LilithHafner
Copy link
Contributor

I tested this locally with about 30 seconds of typing at a rate of 30 characters per second (60 if you count the backspace as a character) when using both short and long queries on the docs.julialang.org search target. I noticed no disruptions that the comments warn about. I was testing on chromium as that browser allows me to edit javascript live, it would be nice to also test on another browser and to hear why @Hetarth02 chose to include these debounces in the first place to make sure this doesn't create instability or performance issues, especially on slow computers and/or mobile (though folks don't type particularly quickly on mobile)

julia> run(`xdotool getmouselocation`)
x:226 y:386 screen:0 window:8388797
Process(`xdotool getmouselocation`, ProcessExited(0))

julia> function f(n)
           run(`xdotool mousemove 226 386`)
           run(`xdotool click 1`)
           for i in 1:n
               run(`xdotool key $(rand('a':'z'))`)
               run(`xdotool key BackSpace`)
           end
       end
f (generic function with 1 method)

julia> f(1000)

This should make search at least 2x faster and, I'd extrapolate, sometimes more than 10x faster (e.g. when there are very few search terms or for packages with small indexes). It entirely removes the 300ms wait between when the user finishes typing and when we begin gathering search results.

Fixes #2411
Supersedes & closes #2407

@Hetarth02
Copy link
Contributor

There are two places for debounce one in filter and another in search box for querying search result.

Filter debounce

  • It is there to prevent multi/mis/double click of more than one filters. In my testing, often times when clicking multiple filters many times in a short span of time it didn't give enough time to the DOM to update and reflect the changes which led to a broken UI.

Search debounce

  • It was there to prevent excessive querying. I assumed that querying the index for each character typed can be a costly operation(especially on lower end devices) and hence put up a delay.

UI/UX reason

  • Secondly, from a UI perspective. If everything happens in a snap sometimes the end user doesn't even recognise that if something has changed or not. Also, in the initial UI the modal had a varying length and not a fixed one hence, when doing anything it would change the size of the modal. You can see if the modal changes the size instantly it may be jarring for some user. Hence this slight delay, somewhat smoothens the process.

Overall, I think we should go forward with reducing the debounce time to what fits for the wider use cases and would recommend not remove the debounce.

@LilithHafner
Copy link
Contributor Author

It is there to prevent multi/mis/double click of more than one filters. In my testing, often times when clicking multiple filters many times in a short span of time it didn't give enough time to the DOM to update and reflect the changes which led to a broken UI.

I clicked three different filters in about 50ms with 6x CPU throttling and everything looked fine without debounce. Further, when I double-click a single button, with or without debounce, there is no impact (i.e. it selects and then deselects). Would you clarify exactly what the problem is that this is preventing (e.g. share a screencast or reproducible instructions for where to click and what to observe)

It was there to prevent excessive querying. I assumed that querying the index for each character typed can be a costly operation(especially on lower end devices) and hence put up a delay.

This seems reasonable. I was actually surprised that removing debounce didn't cause those issues for me, but when I tried again with 6x CPU throttling I did observe that characters typed did not appear instantly when debounce was disabled. This "sluggishness" on slower devices makes this PR not viable as is. I'll look into a wait-free solution that doesn't cause this unresponsiveness.

in the initial UI the modal had a varying length and not a fixed one hence, when doing anything it would change the size of the modal.

This is no longer the case, right?

@LilithHafner LilithHafner marked this pull request as draft January 20, 2024 15:01
@LilithHafner
Copy link
Contributor Author

I'll look into a wait-free solution that doesn't cause this unresponsiveness.

Done! #2415

@LilithHafner LilithHafner deleted the lh/search-debounce branch January 20, 2024 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

De-selecting a search category is sometimes ignored. [bug] [low priority]
2 participants