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

Optimisations on init, filtering, dropdown show/hide after profiling 20,000 options. #1103

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

joshcaspersz
Copy link

I profiled the plugin with 20,000 options and created the following optimisations:
(I've committed each optimisation separately for easy referencing.)

a) Optimised getSelected used in updateButtonText
The jQuery filter was causing a 16 second delay, changing it to the following removed it:
return $(this.$select.get(0).selectedOptions)
selectedOptions is supported in all major browsers except for IE (Edge supports it) so I also implemented a fall back.

b) Optimised buildDropdownOptions
This function was causing the longest delay when loading (nearly 30 seconds).
I've reduced it to under a second by building a string array and then creating the list in one shot.
I disabled dividers and the check for disabled options. You may want to develop further to support these.
Also see XSS fix.

c) Optimised dropdown show/hide
The plugin uses bootstrap drop-downs that toggles the .open style on the dropdown when the button is clicked.
Originally this toggled from display: none to display: block.
Doing so causes a reflow which can take 3 seconds to open and 15 seconds to close on 20000 options.
Toggling visibility:hidden and height:0 instead reduces the delays to about 380ms.

d) Optimised search
This was similar to c), the plugin set display:none or display:block on each list element when filtering.
I removed this and toggled height instead.
This reduced the delay from 1.4 minutes on one search to 800ms.

@dometto
Copy link

dometto commented Sep 15, 2022

@davidstutz any chance this could be merged?

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.

None yet

2 participants