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

Improve performance for big Select #58

Open
codesource opened this issue Feb 18, 2013 · 13 comments
Open

Improve performance for big Select #58

codesource opened this issue Feb 18, 2013 · 13 comments

Comments

@codesource
Copy link

I used your plugin for select with many items (~3000).
Its works but take ~3mins to render with many FF warning about endless loop.

I did some modification and now it take ~6secs.
The idea is first to prepare all items before adding them to DOM.

                optgroupLiTemplate = '<li class="ms-optgroup-label"><span></span></li>',
                containers = {};

                ms.find('optgroup, option').each(function(){

                        ....
                        /*
                        if (optgroupId){
                            that.$selectableUl.children('#'+optgroupId+'-selectable').find('ul').first().append(selectableLi);
                            that.$selectionUl.children('#'+optgroupId+'-selection').find('ul').first().append(selectedLi);
                        } else {
                            that.$selectableUl.append(selectableLi);
                            that.$selectionUl.append(selectedLi);
                        }
                        */

                        var container = optgroupId || 'main';
                        if(!containers[container]){
                            containers[container] = [[],[]];
                        }
                        containers[container][0].push(selectableLi);
                        containers[container][1].push(selectedLi);
                    }
                });
                for(var id in containers){
                    if (id !== 'main'){
                        that.$selectableUl.children('#'+id+'-selectable').find('ul').first().append(containers[id][0]);
                        that.$selectionUl.children('#'+id+'-selection').find('ul').first().append(containers[id][1]);
                    } else {
                        that.$selectableUl.append(containers[id][0]);
                        that.$selectionUl.append(containers[id][1]);
                    }
                }
@lou
Copy link
Owner

lou commented Feb 18, 2013

Hi @codesource,

I'm already aware that appending to the DOM is slow and I think I happen the multiSelect container as late as I can.

The first time I append something to the DOM happens at line 157 (https://github.com/lou/multi-select/blob/master/js/jquery.multi-select.js#L157). Before that, I manipulate elements that are newly created and which are not part of the DOM yet.

I'm curious why your code is more performant.
Could you provide the full source of your change please ?

I know the plugin is not well suited for list with more than 1000 elements and I take this issue seriously.

@codesource
Copy link
Author

I added some comments and an other correction for select without ID.

http://www.code-source.ch/files/jquery.multi-select-patch.js

The reason comes from jQuery core. Each time you call "append" jQuery do more job than only append element to an other.
Refs:
https://github.com/jquery/jquery/blob/master/src/manipulation.js#L108
https://github.com/jquery/jquery/blob/master/src/manipulation.js#L261

Calling on a group of items avoids all these jobs each time.

@venzy
Copy link

venzy commented May 20, 2013

Hi guys,
I applied the patch on top of 0.9.5 (with a bit of fudging to cleanly apply) and experimented with putting some stopwatch logging statements into some lists with 256 elements in them. The most expensive part seems to be the population of the containers arrays (~250ms) compared to appending to the elements (~10ms). I even tried uglifying the code significantly in order to use the string-builder approach to create selectableLi / selectedLi strings and store those in the container arrays instead. Not much difference.
If you can imagine 4 such lists on a page, with the need to refresh them from AJAX data on a regular basis - it pauses the page for about 2 seconds every time!
Not sure where to go from here, as from a quick search this still appears to be the best implementation of a side-by-side javascript multiselect.

@ridiculous
Copy link

How about calling $.detach() on the element before working on it? This should prevent the DOM from "pausing"

@venzy
Copy link

venzy commented May 21, 2013

I'm fairly new to web dev, but my understanding is that the big pauses are
due to the single-threaded nature of JavaScript, and are occurring when we
are working on detached fragments (see previous comment - measurements done
on patched version) rather than at the point where we apply them to the
visible DOM causing styling reflows etc?

On Tuesday, May 21, 2013, Ryan Buckley wrote:

How about calling $.detach() on the element before working on it? This
should prevent the DOM from "pausing"


Reply to this email directly or view it on GitHubhttps://github.com//issues/58#issuecomment-18183913
.

@ridiculous
Copy link

if your seeing a pause when manipulating the DOM, odds are your not working on detached fragments. JavaScript is incredibly fast and looping through a few hundred (or thousand) items in an array isn't costly, however manipulating the DOM is, especially on older browsers. You could also look into WebWorkers for newer browsers

@ridiculous
Copy link

I stand corrected, JS execution is a blocking operation, regardless of DOM manipulation. How about array chunking? Split the array into smaller chunks and have it call itself recursively with a setTimeout (50 ms) in between? This allows the JS process to come to an idle and not freeze the browser.

@bikegriffith
Copy link

Sending a "bump" here as well. See related issue: #101

@ghost
Copy link

ghost commented May 13, 2014

I too would like a suggestion on how to solve loading a list of over 10,000 elements. Perhaps with some sort of ajax triggered pagination?

@ryanbuckley
Copy link

Maybe open a bounty on stackoverflow? That might motivate me to come up with something...

@sdecalom
Copy link

Hi guys,

I have some problem, when i tried to load 2000 items selected, firefox return this message:

too much recursion
...ion(){return!1},m.isXML=function(a){var b=(a?a.ownerDocument||a:0).documentEleme...

This problem occurs on these lines of code

selectables = this.$selectableUl.find('#' + msIds.join('-selectable, #')+'-selectable').filter(':not(.'+that.options.disabledClass+')'),

selections = this.$selectionUl.find('#' + msIds.join('-selection, #') + '-selection').filter(':not(.'+that.options.disabledClass+')'),

Can you help me ?

@codesource
Copy link
Author

I did a fork and added a pull request for that. You can try it.

Firefox + Google chrome:
10'000 items (~4secs).
30'000 items, it works but take ~10secs.
100'000 items, it works but is very slow ~40-70secs

Internet explorer:
10'000 items (~4secs).
30'000 items => timeout.
100'000 items => timeout.

@sdecalom
Copy link

Ha super, it's work fine.

Thank you very munch ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants