https://bugzilla.wikimedia.org/show_bug.cgi?id=34876
--- Comment #3 from Lupo <[email protected]> 2012-03-07 20:13:20 UTC --- Created attachment 10193 --> https://bugzilla.wikimedia.org/attachment.cgi?id=10193 Rewritten jquery.makeCollapsible Short version: the attached rewrite is between two to four times faster, but needs full testing. Long version: Analyzing this more, I think the main problems are that the current implementation: * uses classes to record collapsed/expanded states, * and collapses each of the initially collapsed elements individually, * and just triggers the installed click event handlers to collapse them. Hence many click handlers are invoked. Each one changes some classes, then hides the element. Changing classes may cause page reflows, hiding may inadverently use animations, and triggering the click events causes event bubbling plus other overhead that's neither needed nor wanted in this case. The worst appears to be mixing changing classes with hiding (because hiding stores the computed style, which has to be recalculated after each reflow after each class change), next worst are the animations. The attachment is a rewrite of jquery.makeCollapsible that tries to alleviate these problems without changing the interface. It still uses and changes classes, but it uses a two-phase algorithm for initialization: in a first phase, it just records all the modifications (elements on which classes need to be changed, elements to be hidden), and then in a second phase plays back these recorded changes (making extra sure to avoid animations) at the end. BTW, $.hide() internally uses a similar mechanism: they first record for all elements the computed style and then modify all the elements, instead of doing both steps individually for each element one after the other, as style modifications also may cause reflows. Further optimizations include: * Recording the changes in plain arrays (like data.toggles = []) instead of using initially empty jQuery objects (data.$toggles = $()). The point here is that if we used that and then data.$toggles = data.$toggles.add( $that ), each "add()" call would try to make sure that the resulting set contained only unique elements. We do know, however, that this check is completely superfluous in our case. Avoiding these jQuery.unique() calls that jQuery internally makes gives another significant speedup. This is also why the rewritten code doesn't use $.closest() to find the collapsible element from a toggle as that somehow also calls jQuery.unique(); instead collapsible belonging to a toggle is passed as a parameter (like it was already done for the custom toggles). * Using $.children ( 'foo' ) instead of $.find ( '> foo' ) wherever possible. I had the impression that that also is a bit faster. * Using a (new) function $.changeClass (fromClasses, toClasses) to "atomically" remove some classes and add some other classes instead of $.removeClass( fromClasses ).addClass ( toClasses ). That reduces the number of reflows for class changes by a factor of two. (The function is supposed to take two string arguments, both containing single class names or lists (whitespace-separated) of class names. The implementation does not, like addClass/removeClass, support functions as parameters.) * Not invoking the click handlers to collapse the initially collapsed elements. Even using $.triggerHandler() instead of $.trigger() still had significant unwanted overhead; remembering and then calling the appropriate toggle* function directly is still noticeably faster. The result is still not lightning fast, but in my (admittedly somewhat ad-hoc) tests significantly faster than the current implementation. Of my FF 3.6.27, it's about three to four times faster than the current implementation (fast enough to avoid the "unresponsive script" alerts with the default setting of dom.max_script_runtime); on Chrome, the speedup is about a factor of two. Note that I did test this implementation only manually on Special:Recentchanges; full testing exercising all cases (custom toggles, other stuff) still needs to be done. Testing on Special:Recentchanges actually exercises only the "table" and toggleLinkPremade() paths. The code would also need a unit test for $.changeClass(). Maybe that should even be split out in its own jquery plugin. The rewrite should be functionally identical to the currently existing implementation, with one exception: the new implementation of $.makeCollapsible() returns the subset of elements that were enabled, whereas the current implementation returns the full set passed in. -- Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the assignee for the bug. You are on the CC list for the bug. _______________________________________________ Wikibugs-l mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
