An issue popped up on Twitter this past week that caused the web site to be generally unusable for many users. It appears as if attempts to scroll were unbearably slow and caused the site to be unresponsive.
The Twitter team investigated and determined that if they reverted the version of jQuery that they used back to 1.4.2 from 1.4.4 the site would be responsive again. After more investigation they determined that the code that was slow was doing a contextual selector search for an item by class name, for example: $something.find(".class").
So – what happened? How did this come about? To start, nothing is inherently wrong with jQuery 1.4.4 – this particular performance regression came in jQuery 1.4.3. In 1.4.3 we switched from using the old Sizzle selector engine for contextual queries to using the browser’s native querySelectorAll method, if it exists. This change was even explicitly mentioned and highlighted in the 1.4.3 release notes as it’s a really good change. In general using querySelectorAll will result in much faster queries, especially for complicated queries and complicated documents (which there seem to be a lot of).
However, as with every performance change, while some things get way faster some things can also get slower. This was the case for some previously-optimized queries like .find(“.class”) (where we use getElementsByClassName, if it exists) and .find(“div”) (where we use getElementsByTagName). Both of those aforementioned methods always end up being faster than the queries run through querySelectorAll. Whether this will always end up being the case is another question entirely.
What’s interesting here is that we’ve been using querySelectorAll for our default selector engine in jQuery for quite some time now (doing $(‘.class’) would use querySelectorAll). The only change in 1.4.3 was just filling in a gap where .find(‘.class’) wasn’t using querySelectorAll. We’ve not heard of any particular performance regressions regarding the use of querySelectorAll and $(‘.class’).
This brings up the important point: Just how much faster is getElementsByClassName compared to querySelectorAll? In our preliminary tests it looks like it’s about 0.5-2x faster, depending upon the browser. While this is certainly nothing to scoff at the performance hit of this difference is quite negligible. For example the difference between searching by class name and querying in Firefox 3.6 is about 0.007s – certainly nothing that is capable of crippling a large application.
That being said, we don’t like performance regressions so today we backported some shortcuts into Sizzle (from jQuery) to improve its performance for some common cases. For example: Sizzle(“div”), Sizzle(“.foo”), and Sizzle(“#id”) will all skip using querySelectorAll and try to use the native methods provided by the browser if they exist. (jQuery already did some of these (namely “div” and “#id”, we just added the “.foo” shortcut as well).
So. If the performance hit wasn’t very large then why was Twitter having so many problems? The reality is that this particular change was just the straw that broke the camel’s back. Two things were happening that caused Twitter to have the issues that it was having. These can be distilled down into two general best practices:
Best Practices
It’s a very, very, bad idea to attach handlers to the window scroll event. Depending upon the browser the scroll event can fire a lot and putting code in the scroll callback will slow down any attempts to scroll the page (not a good idea). Any performance degradation in the scroll handler(s) as a result will only compound the performance of scrolling overall. Instead it’s much better to use some form of a timer to check every X milliseconds OR to attach a scroll event and only run your code after a delay (or even after a given number of executions – and then a delay).
Always cache the selector queries that you’re re-using. It’s not clear why Twitter decided to re-query the DOM every single time the scroll event fired, but this does not appear to be necessary (since scrolling itself didn’t change the DOM). They could’ve saved the result of that single query to a variable and looked it up whenever they wanted to re-use. This would’ve resulted in absolutely no querying overhead (which is even better than having the faster getElementsByClassName code!).
Thus, combining these two techniques, the resulting code would look something like:
var outerPane = $details.find(".details-pane-outer"),
didScroll = false;
$(window).scroll(function() {
didScroll = true;
});
setInterval(function() {
if ( didScroll ) {
didScroll = false;
// Check your page position and then
// Load in more results
}
}, 250);
Hope this helps to clear things up and provides some good advice for future infinitely-scrolling-page developers!
"Cowboy" Ben Alman (January 20, 2011 at 4:08 pm)
Note that my very small jQuery Throttle / Debounce plugin can help throttle event handlers. It’s very easy to implement and works great with browser scroll and resize events!
http://benalman.com/projects/jquery-throttle-debounce-plugin/
Dustin Diaz (January 20, 2011 at 4:12 pm)
Thanks John for posting this! For one thing, considering we do it everywhere else, we missed it in this case in regards to caching the selector. I noted that in an update on my blog.
Re: the interval, we did in fact change to that when we downgraded. If you look now in production, we’ve created a 1000ms interval that checks if you’re near the bottom of the page, ditching the ‘scroll’ event altogether.
cheers,
Dustin
Phil Dokas (January 20, 2011 at 4:16 pm)
I was just going to recommend Ben Alman’s excellent throttling plugin, but the man himself beat me to it! It’s great stuff with great docs and clear source. Well well worth your time if you must do actions based on scrolling.
Addy Osmani (January 20, 2011 at 4:17 pm)
Ben has a plugin for every situation :) I kid! I kid. That said, I’m somewhat surprised that Twitter aren’t caching their selectors more.. with what they were doing re-querying the DOM on each scroll event being fired just seems like a recipe for disaster.
Steve Ottenad (January 20, 2011 at 4:20 pm)
Nice writeup, its good to see that concessions are made to accommodate common mistakes by users. You would think that a company as big as Twitter would make it a point to optimize their code though. Thanks for the heads up and the best practices moving forward!
Dave Stein (January 20, 2011 at 4:21 pm)
Can’t tell you how many times I’ve had to tell interns to cache their selectors. Don’t think any full time programmers here forget – esp on a repeated event. Funny the twitter guys missed out on something so simple.
Schalk Neethling (January 20, 2011 at 4:30 pm)
Great information John, thanks. Do you know if Twitter is going to implement this?
Michael Jackson (January 20, 2011 at 4:33 pm)
Another technique that I’ve used in the past is to do the following:
var timer = 0;
$(window).scroll(function () {
if (timer) {
clearTimeout(timer);
}
// Use a buffer so we don't call myCallback too often.
timer = setTimeout(myCallback, 100);
});
Instead of setting up an interval, the callback is only ever fired when it needs to be. Saves a few extra calls to the callback, and any overhead from the extra logic inside the scroll handler should be fairly minimal. I’d be curious to know how you think this strategy compares.
Dan Heberden (January 20, 2011 at 4:34 pm)
@Schalk, Dustin Diaz (@ded) is an engineer at twitter
Nicholas C. Zakas (January 20, 2011 at 4:43 pm)
Also good to call out differences in performance between getElementsByTagName(), querySelectorAll() and getElementsByClassName(): http://www.nczonline.net/blog/2010/09/28/why-is-getelementsbytagname-faster-that-queryselectorall/
Static nodelists vs. live nodelists is a topic that deserves more discussion and certainly more understanding.
John Resig (January 20, 2011 at 5:08 pm)
@Michael: Yep – that’s also a good technique!
@Nicholas: Good recommendation on that topic! I don’t think it applies in this particular case (as anytime we call the native methods we instantly turn the results into a static array) – thus losing any lazy-access benefit that it might normally yield.
Andreas Grabner (January 20, 2011 at 5:23 pm)
Great post. We have also posted some findings on selector performance – hope its helpful:
http://blog.dynatrace.com/2009/11/09/101-on-jquery-selector-performance/
http://blog.dynatrace.com/2010/08/25/top-10-client-side-performance-problems-in-web-2-0/
Cheers
Josue Auguso (January 20, 2011 at 6:10 pm)
> It’s a very, very, bad idea to attach handlers to the window scroll event.
Another signal of incompetence of twitter? Color me shocked.
Franz Buchinger (January 20, 2011 at 6:10 pm)
Interesting workaround for “hyper-nervous” event listeners. However I’m not entirely happy with the presented techniques. setTimeout has scoping issues and doesn’t integrate at all in my jQuery chains. Ben Alman’s plugin doesn’t seem to allow inline event handlers.
The most elegant approach would be if jQuery’s event system came with throtteling/muting/debouncing capabilities, maybe via .bindDebounced/.bindThrotteled methods:
$(window).bindDebounced('scroll', 250, function(){//event handler})
ian (January 20, 2011 at 6:15 pm)
great post, love the deep dives into the core. thanks for sharing!
RobG (January 20, 2011 at 6:36 pm)
@Zakas
Interesting that one of the arguments for qSA returning static lists was that it would be faster than returning static lists. It was argued at the time that that opinion was not necessarily correct (including a Mozilla developer) and that it was more important to have a consistent API, so qSA should return live lists. It now appears that indeed there is not necessarily any performance penalty in returning a live node list vs a static one.
It may be worth considering that at the time the qSA spec was being developed, the “major” libraries returned static lists for selector queries. There were a number of developers of such libraries influencing the development of the qSA spec, clearly they had a vested interest in making qSA lists static to maintain consistency with their APIs at the expense of a consistent DOM API.
They are hoist by their own petard.
Kyle Hotchkiss (January 20, 2011 at 7:01 pm)
I still think it’s cool that Twitter uses jQuery anyways =D
Dan Lash (January 20, 2011 at 8:19 pm)
Any chance that this performance regression gives you second thoughts about the “slow .add() in ie” bug? True it is a ‘correct’ solution, but having per-browser code is better for performance – at least for IE.
http://bugs.jquery.com/ticket/7341#comment:10
John Resig (January 20, 2011 at 8:57 pm)
@RobG: I’m not sure which library dev(s) you are referring to but it’s none that I know of. As I understand it most of the library devs (such as from Prototype or jQuery) were invited very late to the standardization process (after implementations had already started to land in browsers). I know that I, personally, would’ve loved a live list (it would’ve allowed us to create some really interesting an expressive APIs) but was told by various browser implementers that it just wasn’t possible. *shrug* The best we can hope for is to simply create some better APIs going forward!
Jeff Waugh (January 20, 2011 at 9:03 pm)
Great post, John — thank you!
Funny, I wrote the following code just the other night. Is this going too far to hide timer in a closure? :-)
$window.resize( (function() {var timer = null;
return function(e) {
if (!timer) timer = setTimeout(function() {
fixFontSize();
timer = null;
}, 250);
};
})() );
Garrett (January 20, 2011 at 9:08 pm)
Zakas blogged about event throttling about three years ago on one of Yahoo blogs, using virtually the same approach, only with the “resize” event.
And Cornford has advocated also the approach for mousemove on c.l.js.
However, another approach, and one that does not require the use of
setInterval, is to throttle the callback firing only after a minimum number of milliseconds has elapsed. THis is done simply by using+new Date.Copy’n’pasted from a thread:
http://groups.google.com/group/comp.lang.javascript/browse_thread/thread/bc5274b75820e5ff/96fbac718201ebbf#96fbac718201ebbf
| I also use a MOUSE_MOVE_THRESHOLD to
| reduce CPU strain in mousemove.
|
| var now = +new Date;
| if(now – lastMouseMoveTime < MOUSE_MOVE_THRESHOLD) return;
lastMouseMoveTime = now;
That logic would seem to be nicer in another function. In theory, it should be a bit more efficient than setInterval and multiple timers — setInterval or setTimeout — can add overhead. OTOH, (+new Date) overhead is virtually nothing.
Jeff Waugh (January 20, 2011 at 9:29 pm)
Garrett: Do you suggest
+new Datebecause it’s faster thanDate.now()?Jeff Waugh (January 20, 2011 at 9:35 pm)
Good answer to my question here: http://jsperf.com/new-date-value/3
“Date.now() is the fastest. It’s more than 50% faster than +new Date. Too bad IE8 and below don’t support it.”
Dan (January 20, 2011 at 9:52 pm)
Mr. Resig,
I believe we could use Michael’s idea of only calling the timeout when necessary, while also avoiding the need to call clearTimeout hundreds of times per scroll.
var outerPane = $details.find(".details-pane-outer"),
didScroll = false,
timer = false;
$(window).scroll(function() {
if(!didScroll) {
timer = setInterval(function() {
if ( didScroll ) {
didScroll = false;
clearTimeout(timer);
// Check your page position and then
// Load in more results
}
}, 250);
}
didScroll = true;
});
Paulo Eduardo Neves (January 20, 2011 at 11:15 pm)
Great timing. I’ve just searched for fixed pane and found this StackOverflow answer that suggests me to do it wrong.
Ron Korving (January 21, 2011 at 1:39 am)
@John Resig
Dynamic node lists (through getElementsByClassName) would definitely help vs. querySelectorAll, because the former doesn’t re-iterate your document trying to find the nodes you are looking for. Calling getElementsByClassName a thousand times only does the document iteration once. After that, those calls are blazing fast.
Ryan Kirkman (January 21, 2011 at 1:40 am)
@”Cowboy” Ben Alman underscore.js has a throttling function too: http://documentcloud.github.com/underscore/#throttle
Anthony Bowyer-Lowe (January 21, 2011 at 7:14 am)
Great stuff.
It always feels to me that modern JS programming is a throwback to old-school micro-computer game & demo coding where amazing things are possible when smart thinking and optimisations are performed. Mostly that means truly understanding the cost of things, avoiding all unnecessary work (lazy code!), and intelligently pre-calculating and caching everything where it makes sense (and doesn’t unduly blow out memory).
Its time to pass these techniques down to the new generation of coders so we can all benefit from the amazing potential of in-browser progress.
Balázs Galambosi (January 21, 2011 at 8:11 am)
@Jeff Waugh – Think about it. Even for the slowest device (iPhone) any of the time methods are able to perform 100 000 operations / second.
That’s 1/100 ms for one call LTN (less than nothing).
You can use this general function btw for throttling.
var throttle = (function () {
return function (fn, delay) {
delay || (delay = 100);
var last = +new Date;
return function () {
var now = +new Date;
if (now - last > delay) {
fn.apply(this, arguments);
last = now;
}
};
};
})();
And then it can be used like this:
var outerPane = $details.find(".details-pane-outer"),
$(window).scroll(throttle(function() {
// Check your page position and then
// Load in more results
}, 250));
As general as it gets.
Robert Kati? (January 21, 2011 at 8:21 am)
The Debounce plugin by Ben fits very well here, indeed.
However in cases where the event is fired very intensively, clear/setTimeout calls can slow down all the thing.
I come with a less clear/setTimeout intensive debounce: https://gist.github.com/789582
Here is a performance test: http://jsperf.com/my-debounce
Note that this test case probably performs debounceing more often than it would be in real life.
"Cowboy" Ben Alman (January 21, 2011 at 8:22 am)
@Garrett, for throttling, I advocate using a combination of elapsed time and a timeout. While the elapsed time is useful to limit execution while the throttling function is being invoked, it is not capable of executing the throttled function “one last time” after the last throttling function invocation. This “one last time” behavior is most often necessary when the function being throttled is setting state. See the graph and description of the “no_trailing” argument in the throttling section of this page:
http://benalman.com/projects/jquery-throttle-debounce-plugin/
@Ryan yes, in fact, someone just recently petitioned them to include some of the other throttle / debounce options I make available in my plugin in Underscore.js!
https://github.com/documentcloud/underscore/issues/91
eduardocereto (January 21, 2011 at 8:56 am)
[OFF-TOPIC]
Srsly John fix your RSS feed. I don’t want to miss this kind of discussion again.
LX (January 21, 2011 at 9:49 am)
We had a similar issue when we used the scroll event to reposition certain stuff in IE8: the repositioning kept switching the scroll bar on and off, thus toggling the scroll event to create an infinite loop which crashed the browsers in less than a second.
BTW.: An “afterscroll” pseudoevent would be really neat for such cases.
Greetings, LX
Oleg Slobodskoi @oleg008 (January 21, 2011 at 10:01 am)
@John this scrolling event problem as old as my grandmother :) every one who uses ‘scroll’ or ‘resize’ event should have noticed this performance issue.
Why don’t to fix that in jquery core? Its just about couple lines of code.
F.e. jquery could provide an util function which will enable to call any function only once in xxx ms.
I am using this tiny implementation:
https://gist.github.com/789765
khs4473 (January 21, 2011 at 10:15 am)
The regression probably had at least as much to do with the fact that for rooted QSA, Sizzle assigns and then de-assignes the ID attribute. For $(“x”).find(“[y]”), Sizzle will be assigning and de-assigning the ID for every element matched by the selector “x”.
Mario (January 21, 2011 at 10:18 am)
Are there some thoughts or work related to separating interface processes/threads from other kind of code in Web Apps? What if someone needs to have both a very responsive UI (that is doing something with the user scrolling) and a responsive application on the other side? Could it be solved with WebWorkers?
Poringo (January 21, 2011 at 10:51 am)
Changing my infinite scroll code right now, thanks for this advice :D
John Resig (January 21, 2011 at 11:37 am)
@eduardocereto: Fixed it just a couple hours ago. Sorry for the hassle!
@Oleg: It’s certainly something to consider – although, as previously mentioned, there are plenty of jQuery plugins that tackle this issue in very constructive ways (such as the debounce plugin).
@khs4473: Nah, didn’t have much to do with it. As you can see by the performance numbers jQuery 1.5 is comparable to, if not faster than, jQuery 1.4.2 and .find(“.class”). The code in 1.5 is still doing the ID attribute changing so that’s not the issue here.
khs4473 (January 21, 2011 at 12:15 pm)
@john Yeah, but now you’re fast-tracking “.class”, “tag”, etc. – bypassing the ID attribute manipulation (and QSA) – which is awesome. But is 1.5 faster even for non-fast-tracked queries i.e. .find(“[attr=x]”), where you’re still using rooted QSA?
Stuart McFarlane (January 21, 2011 at 12:16 pm)
Anyone ever written a device driver interrupt handler? Old school meets new school.
khs4473 (January 21, 2011 at 12:31 pm)
@john Just ran some of my own tests and you’re right of course – QSA is *far* slower than getElementsByClassName – good to know!
mahdi (January 21, 2011 at 12:48 pm)
Thanks john,
scroll event just killed their website,
so just to be clear are we using getElementByClassName or querySelectorAll in jquery 1.4.4 ?
Oleg Slobodskoi @oleg008 (January 21, 2011 at 1:56 pm)
I just have took a look at throttle and debounce implementations, changed a bit the api, and made some optimizations, it looks now perfect for me and can be shiped with jQuery right now :)
https://gist.github.com/789765
josh (January 21, 2011 at 5:57 pm)
@benallman You’re debounce plugin seems like a lot compared to a couple of lines. What is the reason a developer would choose to import a plugin rather than just use either John’s or Michael’s 3-line solutions? Not trying to be rude, I’d really like to know.
Joss Crowcroft (January 21, 2011 at 11:42 pm)
Fantastic post John, Had never occurred to me about the pitfalls of binding to the scroll event!
Balázs Galambosi (January 22, 2011 at 7:02 am)
@Joss – Micheal’s code is debounce not throttle. Which means it will execute after the scroll event hasn’t fired for X ms.
This is not ideal in this case, because if someone reaches the end of the page and keeps scrolling the callback isn’t called.
As to John’s code, it’s not general enough. It polls with a setInterval every 250ms. A general solution does nothing if there’s no action, and so can be used many times for different handlers on the same page.
This is why a general solution is always more lines of code than a specific one.
Tobias Bu (January 22, 2011 at 8:10 am)
The plugin from ben is a good thing.
I use somesing similiar in my experimental library:
https://github.com/nuxodin/The.js
Using like this:
window.onscroll = function(){
foo()
}.ignoreUntil(250):
Robert Katic (January 22, 2011 at 5:11 pm)
In cases where you need an “afterscroll” then debounce is obviously what you wont. If you also have to handle “scroll” every X ms, then I suggest something like https://gist.github.com/789582#file_silencer_example.md
Robert Katic (January 22, 2011 at 6:52 pm)
PS. I was only considering an more generic solution, but in simpler cases like detecting scroll event, the John’s solution with setInterval is best.
David Oster (January 27, 2011 at 5:15 am)
Certainly not a jQuery expert but I was wondering why can’t we use jQuery thread to execute intensive node calls and then post back to the line of showing the nodes?
http://plugins.jquery.com/project/thread