Go Go Go Go Chosen (Chosen gets a speed boost) #1339

pfiller · 2013-07-08T20:05:49Z

Better get ready because this is a big one.

Chosen has notable speed problems when running with lots and lots of lots of items. Though Chrome can be quite forgiving, there is still a lag when thousands of items are in a list. Forget about using IE8 with thousands of items.

There exists an obvious performance problem with Chosen's current winnow_results method. For each item in a list, Chosen does a DOM manipulation (to show or hide the element). Each manipulation triggers a reflow which isn't such a big deal for a list of 10 ... but you can see how that would quickly cause Chosen to become sluggish.

In this PR, the winnow_results method just loops through the results_data array and marks each result as a match or not. After the results are properly marked, results_option_build is called and the search results html is re-built. That is, we're replacing n dom manipulations with just 1.

I bet you want stats. For a select with 1,000 options:

Applying Chosen is 62% faster across all browsers
Showing the search results for the first time is 70% faster across all browsers.
Showing the search results for the first time is 92% faster in IE8. 92%!!!!!
Searching Chosen is 77% faster.

If we bump the list to 5,000 options:

Applying Chosen is only 41% faster.
Showing the search results for the first time is 89% faster across all browsers.
Search Chosen is 72% faster

To get these numbers, I created an HTML page that created a list and ran Chosen 100 times (50 single and 50 multiple) and then I ran it against the new version and the old version. I've shared the test file and the results for those curious.

I don't think these numbers should be treated as scientific, but I do think they are honest. You only need to open a select with 5000 items once in the two different versions of Chosen before you recognize the difference. Chosen is absolutely faster and it feels it.

Now, this is a big change and I would appreciate your careful review. I've done a lot of click testing, but I would love some more. This is a real nice change for Chosen, though, and I'd love to see it land this week.

Thanks!

Conflicts: chosen/chosen.jquery.min.js chosen/chosen.proto.min.js coffee/chosen.jquery.coffee coffee/lib/abstract-chosen.coffee public/chosen.jquery.js public/chosen.proto.js

Conflicts: public/chosen.jquery.js public/chosen.jquery.min.js public/chosen.proto.min.js

Conflicts: public/chosen.jquery.min.js public/chosen.proto.min.js

These are dom-based functions which add and remove classes to show option availability. Our new options_build method makes them unecessary.

In this new version, the DOM is recreated every time the serach filter changes. That means the element that @result_highlight refers to is gone after a search.

new variable to track match status. Also: change value to true if a parts match is found.

results are currently showing. results_show calls winnow so there's no need to do this in advance.

This is the only part of winnow_results that is jQuery dependent and moving it paves the way to move winnow_results into AbstractChosen.

It should have been this way from the beginning, but it wasn't. Moving to a method will move results_otion_build to AbstractChosen

single_set_selected_text method. They are always called together so it makes sense to link them more tightly. Note: test for @allow_single_deselct is present in the single_deselect_control_build method itself - so no need for redundancy.

3d6173a to Prototype version

Conflicts: public/chosen.jquery.min.js

setting the search results text. .html() is jQuery-only and has no place in AbstractChosen.

See d82d344

winnow_results needs to be able to update content and this will contribute to moving it to AbstractChosen

pfiller · 2013-07-08T20:08:00Z

I probably should have mentioned: because we are no longer doing dom manipulation in winnow_results, it's been moved into AbstractChosen. A few helper methods have been added to the proto and jquery methods which handle some smaller dom stuff.

koenpunt · 2013-07-08T22:01:17Z

Oh wow, this is big one. But great work. 👍

Conflicts: public/chosen.jquery.js public/chosen.jquery.min.js public/chosen.min.css public/chosen.proto.js public/chosen.proto.min.js

kenearley · 2013-07-10T16:00:27Z

coffee/lib/abstract-chosen.coffee

@@ -68,7 +86,7 @@ class AbstractChosen

    style = if option.style.cssText != "" then " style=\"#{option.style}\"" else ""

-    '<li id="' + option.dom_id + '" class="' + classes.join(' ') + '"'+style+'>' + option.html + '</li>'
+    '<li id="' + option.dom_id + '" class="' + classes.join(' ') + '"'+style+'>' + option.search_text + '</li>'


It would be nice to get rid of the concatenation and use CoffeeScript's interpolation syntax here.

I actually think the interpolation version is harder to read:

"<li id=\"#{option.dom_id}\" class=\"#{classes.join(' ')}\"#{style}>#{option.search_text}</li>" '<li id="' + option.dom_id + '" class="' + classes.join(' ') + '"'+style+'>' + option.search_text + '</li>'

They're both ugly, though.

I like less quotation marks.

It is possible to use triple quotes like this:

"""<li id="#{option.dom_id}" class="#{classes.join(' ')}"#{style}>#{option.search_text}</li>"""

kenearley · 2013-07-10T16:29:33Z

This is great. I just have the one comment about string concatenation, which is not a blocker.

Speed Chosen up significantly.

- Speed Chosen up significantly (#1339) - Fix broken RTL example #1331 - Updated release procedure to remove S3 #1323 - Include docsupport contents in zip #1321 - Replace deprecated grunt-css with grunt-contrib-cssmin #1319

pfiller added 29 commits June 3, 2013 17:39

Try building a giant html block and see how speed is affected.

2966118

Merge branch 'master' into the-need-for-speed

75a2292

Conflicts: chosen/chosen.jquery.min.js chosen/chosen.proto.min.js coffee/chosen.jquery.coffee coffee/lib/abstract-chosen.coffee public/chosen.jquery.js public/chosen.proto.js

Merge branch 'master' into the-need-for-speed

1a3850c

Conflicts: public/chosen.jquery.js public/chosen.jquery.min.js public/chosen.proto.min.js

Make sure selected elements get flagged on build

3750502

Remove a bogus console call.

d8b7ba7

Merge branch 'master' into the-need-for-speed

ff66c28

Conflicts: public/chosen.jquery.min.js public/chosen.proto.min.js

Remove a bunch of unused winnow code

99f5a1b

Remove result_activate & result_deactivate functions

79b2af6

These are dom-based functions which add and remove classes to show option availability. Our new options_build method makes them unecessary.

Always set the result highlight after results are winnowed.

e662ca0

In this new version, the DOM is recreated every time the serach filter changes. That means the element that @result_highlight refers to is gone after a search.

Group results should be visible like any other search result.

665224a

Clear search results area if no results are found.

675c17b

Rely on an option's search_match value instead of creating a

a5ead6d

new variable to track match status. Also: change value to true if a parts match is found.

After deselecting a multilpe option, winnow the results only if

29f9337

results are currently showing. results_show calls winnow so there's no need to do this in advance.

Search multiple options even if they are selected.

2ff7390

Move search text value conditional out of winnow_results

d82d344

This is the only part of winnow_results that is jQuery dependent and moving it paves the way to move winnow_results into AbstractChosen.

Always set selected text and classes in a method.

3d6173a

It should have been this way from the beginning, but it wasn't. Moving to a method will move results_otion_build to AbstractChosen

Move single single_deselect_control_build into

b5ce559

single_set_selected_text method. They are always called together so it makes sense to link them more tightly. Note: test for @allow_single_deselct is present in the single_deselect_control_build method itself - so no need for redundancy.

Move results_option_build function into AbstractChosen

671147f

Add single_set_selected method outlined in

0aef565

3d6173a to Prototype version

Merge branch 'master' into the-need-for-speed

47b4078

Conflicts: public/chosen.jquery.min.js

Refactor results_option_build to return content instead of

dc094da

setting the search results text. .html() is jQuery-only and has no place in AbstractChosen.

Update results_build to use new content building function

7ae2325

Add a get_search_text method to the Prototype version

2c68aa8

See d82d344

Use an abstracted function for setting search results content.

1a7cd9e

winnow_results needs to be able to update content and this will contribute to moving it to AbstractChosen

Make update_results_content availabel in Prototype version

542554e

winnow_results is now framework independent. Move it to Abstract

2bc288d

result_activate and result_deactivate are no longer needed

89e584c

Always set result highlight after winnowing search results.

6821232

Only winnow results after deselct if results are showing.

c35b81e

Merge branch 'master' into the-need-for-speed

95daa8a

Conflicts: public/chosen.jquery.js public/chosen.jquery.min.js public/chosen.min.css public/chosen.proto.js public/chosen.proto.min.js

kenearley reviewed Jul 10, 2013
View reviewed changes

This was referenced Jul 10, 2013

Speed up rendering for large (15k+) selects #512

Closed

Speed up of Chosen.winnow_results() method #522

Closed

Triple quotes allow for string interpolation

d741861

pfiller added a commit that referenced this pull request Jul 10, 2013

Merge pull request #1339 from harvesthq/the-need-for-speed

992cf28

Speed Chosen up significantly.

pfiller merged commit 992cf28 into master Jul 10, 2013

pfiller deleted the the-need-for-speed branch July 10, 2013 18:39

This was referenced Jul 10, 2013

Chosen.prototype.winnow_results when searchText is empty #1139

Closed

Speed of chosen #1203

Closed

Don't winnow results when search is empty #1226

Closed

Performance issue with thousands of items #579

Closed

tjschuck mentioned this pull request Jan 6, 2015

Allows searching for & and other special characters #2157

Closed

pfiller mentioned this pull request Mar 4, 2015

Large number of <option>'s slows down other page functions. #2158

Closed

koenpunt mentioned this pull request Apr 29, 2017

Replace deprecated CSS clip property #2780

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Go Go Go Go Chosen (Chosen gets a speed boost) #1339

Go Go Go Go Chosen (Chosen gets a speed boost) #1339

pfiller commented Jul 8, 2013

pfiller commented Jul 8, 2013

koenpunt commented Jul 8, 2013

kenearley Jul 10, 2013

pfiller Jul 10, 2013

pfiller Jul 10, 2013

kenearley Jul 10, 2013

koenpunt Jul 10, 2013

pfiller Jul 10, 2013

pfiller Jul 10, 2013

kenearley commented Jul 10, 2013

Go Go Go Go Chosen (Chosen gets a speed boost) #1339

Go Go Go Go Chosen (Chosen gets a speed boost) #1339

Conversation

pfiller commented Jul 8, 2013

pfiller commented Jul 8, 2013

koenpunt commented Jul 8, 2013

kenearley Jul 10, 2013

Choose a reason for hiding this comment

pfiller Jul 10, 2013

Choose a reason for hiding this comment

pfiller Jul 10, 2013

Choose a reason for hiding this comment

kenearley Jul 10, 2013

Choose a reason for hiding this comment

koenpunt Jul 10, 2013

Choose a reason for hiding this comment

pfiller Jul 10, 2013

Choose a reason for hiding this comment

pfiller Jul 10, 2013

Choose a reason for hiding this comment

kenearley commented Jul 10, 2013