Bug HuntWhat made this blog slow?
A while ago the blog start taking 100% CPU on the client machines. Obviously we were doing something very wrong there, but what exactly was it?
We tracked down the problem to the following code:
As you can probably guess, the problem is that we have what is effective an infinite loop. On any Ajax request, we will generate a new Ajax request. And that applies to our own requests as well.
The fix was pretty obvious when we figured out what was going on, but until then…
More posts in "Bug Hunt" series:
- (30 Jan 2012) What made this blog slow?
- (27 Jan 2012) What made this blog slow?
Comments
In JavaScript you should always use braces on if-statements.
@Philipp That's more like a convention.
@Phillip: Maybe you should lookup the meaning of those curly braces in c-style languages. An if statement can only execute a single instruction depending whether the condition is true or not.
With curly braces you declare an instruction block. When you combine curly braces with conditional language structures you can execute multiple statements. Think of it if you we're declaring an anonymous method with those curly braces and assign that to the true delegate of the if statement.
@Dave: What are you getting at? I'm pretty sure Phillip knows how conditionals work. Using braces after a conditional is a good convention to follow.
Even though this isn't the point of Ayende's article at all, Phillip is correct. While braces are not required in JavaScript, they would be considered a hard stylistic rule. If you read Crockford's book on the language you can get the details as to why this is the case. In short, there are certain scenarios where not having the braces can actually lead to incorrect code interpretation...leading to extremely difficult to detect bugs. In other words, there are bugs in the JavaScript language that surface when not using braces...so just use braces everywhere instead of trying to memorize the particular edge cases.
@Dave You actually make one of the classic JavaScript mistakes, which is to assume that because it looks like a c-style language, that it actually is. This is not the case and there are many more examples of this than the braces issue, many of which can lead to very hard to find bugs. You have to treat JavaScript as its own language regardless of what it looks like.
More importantly, when you DO use curly braces (which you should, it's less ambiguous) make sure you consistently format them K&R style, because in javascript, brace placement matters (http://encosia.com/in-javascript-curly-brace-placement-matters-an-example/).
Why subscribe to a global event in the first place?
@dyu, @Dave, actually in the case of Javascript it's more than just convention. Doing it can help prevent bugs.
Reference:
http://www.youtube.com/watch?v=hQVTIJBZook
I'd be very curious to know how this code got published to a production environment without getting caught. I would have thought this problem would have occurred in any testing environment as well as it did here. Ayende, can you comment on where the process broke down and how such an obvious bug was able to slip through?
@Rob:
A better recommendation would be to not use JavaScript in the first place. One really wonders how this junk ever got accepted.
@configurator:
Ajaxy-pagination, page composition etc. It would be a PITA to wire each one up individually.
@ThomasW,
That's because of a bug/feature in the way browsers implement javascript. Just like browsers do a half assed job of implementing all sorts of standards. AFAIK Other implementations of ECMAScript don't have this problem.
Anyway a half decent IDE will give you a warning on this. What is the big deal?
Actually Joao this isn't an implementation problem, it's a Javascript problem. Check the spec out for yourself, http://www.ecma-international.org/publications/files/ECMA-ST/Ecma-262.pdf page 26.
Time for everyone to just admit that JS is a terrible language and move on. Stop blaming implementations for problems with a shit language.
Martin, to what client-side webpage language do we move on to? What is the alternative?
Comment preview