16 ms·
Node.js: Some quick optimization advice
- spullara 11y agoWow, the v8 optimizer can sometimes be a pretty blunt instrument. Shouldn't it look at the size of the inlined function after it compiles it to some intermediate state that erases things like comments and names?
- striking 11y agoSuch a state does not exist inside V8. Things happen as they are parsed. Since V8 has no idea what a piece of JS will do until it gets all the context, it has to use a silly thing like character count to perform inlining. It could create an intermediate state, but that would require a code overhaul that would likely slow down the entire thing anyway. If I had to suggest a change, I would use the 600-char count as a starting point for one-time functions, and then count the number of calls to a function, counting how many instructions it produces (basically tossing an incrementing variable into the compilation state, and storing it per Function object). But then again, I've never played with the inside of V8, only the outside. I consider myself lucky to be in that position, because V8 is pretty dang sweet.
- nostrademons 11y agoNo, V8 parses your Javascript source into an AST. From the AST, there are two paths it can take. The first step is the "full-codegen" compiler, which walks the AST directly and emits unoptimized native code: http://wingolog.org/archives/2011/07/05/v8-a-tale-of-two-compilers http://wingolog.org/archives/2011/07/05/v8-a-tale-of-two-com... If a function is hot, then it gets promoted to V8's optimizing compiler, which is currently Crankshaft but will soon be replaced by Turbofan. This compiles the AST first into a high-level IR, Hydrogen, which is an architecture-independent SSA-like form vaguely reminiscent of LLVM. Then that's lowered into an architecture-dependent IR, Lithium, where register allocation and instruction scheduling is performed. http://jayconrod.com/posts/54/a-tour-of-v8-crankshaft-the-optimizing-compiler http://jayconrod.com/posts/54/a-tour-of-v8-crankshaft-the-op... If you read the bug for this that someone posted up-thread, the AST information is fully available when inlining in Hydrogen, but a patch to remove the character limit tanked one of their benchmarks. Also, they didn't preclude fixing it in Turbofan, only in Crankshaft, and that's largely because Crankshaft is on its way out. (I've played a little with the internals of V8, but am not an official developer. Also, one of my friends is the Bay Area TL for V8.)
- alkonaut 11y ago> Things happen as they are parsed. If comments aren't completely ignored by the parser, then there has to be some case where a comment inside a function has an observable effect?
- ycitm 11y agoYup, see @pauljz's comment above - https://news.ycombinator.com/item?id=10375297 https://news.ycombinator.com/item?id=10375297
- alkonaut 11y agoI still don't understand that. Function.prototype.toString() wouldn't break because the parser stripped the comments, it would just output the source without comments? Does any code anywhere depend on the comments being preserved?
- spullara 11y agoPeople literally toString functions and parse them to implement features. Those features depend on there being comment text in the function that they parse. Pretty much a terrible hack but it is true that you can't remove the comments without breaking current code.
- alkonaut 11y agoWow. That's just something that should be an obvious fix in a future ES. Not only does it hurt the optimizer, it also seems to imply that the content of comments sometimes has semantic meaning in the code, such as people having invented directives etc. Surely this must be undocumented features of the language spec?
- chrisseaton 11y agoAre you sure that the size of the intermediate representation is a good indication of suitability to inline? I think we'd be replacing one arbitrary heuristic with another. If we wanted to do really well we'd look at the cost of the actual operations in both the caller and callee, and reason about how they interact with inlining and things like that, but that's a bottomless pit of a problem and by then the user has browsed to a different page.
- hyperpape 11y agoHotspot and V8 serve very different niches, but doesn't Hotspot use the size of the bytecode for this decision? (Or rather, that's the first step of deciding. It will inline larger methods if they're sufficiently hot).
- spullara 11y agoTrue, just a better heuristic (Hotspot uses it). Not clear that with javascript you have time to do a better job. Even if it was done in the parser I think you could make a better size counter that didn't include comments and names.
- pauljz 11y agoIf you're wondering why comments are a factor at all, and aren't just discarded by the lexer, remember that comments in JS are preserved and available to things like `Function.prototype.toString()`. I've seen this used to do evil multiline string support a few times. Slap the multiline string or template into a comment inside a function and then have another function that toStrings it and strips the boilerplate. This sounds like a horrifying hack but it's also pretty similar to how Angular's shorthand DI works.
- albemuth 11y ago> This sounds like a horrifying hack (...) how Angular's shorthand DI works.
- clessg 11y ago> I've seen this used to do evil multiline string support a few times. Love me some FOAM: https://github.com/foam-framework/foam/blob/master/apps/todo/js/com/todomvc/Todo.js https://github.com/foam-framework/foam/blob/master/apps/todo....
- Dru89 11y agoThat might be one of the scariest things I've looked at all day.
- clessg 11y agoIt only gets better. https://github.com/foam-framework/foam/blob/master/apps/todo/js/com/todomvc/Controller.js https://github.com/foam-framework/foam/blob/master/apps/todo...
- phpnode 11y agoit's incredible that that project has 11,000 commits, a huge amount of effort has gone into it but the code is really unpleasant to read. I feel like I must be missing something about FOAM - who is using it and why?
- constexpr 11y agoThe bug has been marked as WontFix by the V8 team: https://code.google.com/p/v8/issues/detail?id=3354 https://code.google.com/p/v8/issues/detail?id=3354. It looks like they are considering fixing it for TurboFan and not for Crankshaft, but I'm not sure what that means. Does that only apply to asm.js code?
- bananaoomarang 11y agoI think Turbofan aims to supersede Crankshaft, but gradually.
- oso2k 11y agoTurbofan is an additional layer of optimization after Crankshaft.
- WorldWideWayne 11y agoSo, is this is a point in favor of not commenting your Javascript code or using inline docs? Historically, I preferred to use inline JsDoc style comments as the source of documentation for my public APIs. Recently though, I decided that I didn't like them and that I wanted something better. I was hoping to find some tool that parses my JS to AST, figures out what was being exported (e.g. what was public) and writes a JSON document that I could diff over time, to figure out what was documented and what wasn't (in my external docs). So far, I have found this project called `doctor` which sounded close to what I'm looking for, but it still relies on comments ~ https://github.com/jdeal/doctor https://github.com/jdeal/doctor ~ So, I might have to write it myself using something like Esprima to get the AST...
- madflame991 11y ago> So, is this is a point in favor of not commenting your Javascript code or using inline docs? You can of course strip comments when making builds. In fact you can even write functions of more than 600 chars and get away with it as long as you use a minifier. If you have a minified function of more than 600 chars then that's a bulky function which should instead be split in smaller ones.
- xlm1717 11y agoI wonder what performance gains/losses you would experience if you had the comment outside the function block, before the function declaration.
- STRML 11y agoExactly, if the comment were outside the function, there would be no issue.
- WorldWideWayne 11y agoIf the function is nested within another function, then the comments to document the inner function are going to exist inside the outer function. And having nested functions is a very common scenario in JS.
- m1sta_ 11y agoI had this exact thing happen to me. It drove me crazy. Thank you!
- tambourine_man 11y agoPosted that yesterday, didn't get much traction. Whenever I begin to convince myself that JS and Node are actually fine languages / environments to code on, some weird edge case like this pops up. Not entirely sure if it's just its popularity, which is bound to expose its rough corners, or if it's fundamentally bad designed (written in 10 days in the late 90's, etc)
- xlm1717 11y ago>written in 10 days in the late 90's Netscape browser has not been used in a long time, and JS has been standardized as ECMAScript since the late 90's.
- nikolay 11y ago"Standardized" does not equate "good" though. ECMAScript is a very confused language that keeps borrowing from other languages and turns itself into a mess. Not to mention that "Universal JavaScript" is an oxymoron as most real-world JavaScript (well, ECMAScript) is not supported by major browsers and developers need to use things like Babel and the likes. And I don't see an end to it.
- abritinthebay 11y ago> a very confused language that keeps borrowing from other languages and turns itself into a mess I could describe almost any language that builds on previous language idioms like this. C++, most of the later Lisps, Java for sure, C# especially, Objective-C certainly... etc. You're assuming your conclusion then trying to prove it with personal opinion.
- nikolay 11y agoWell, JavaScript became popular, because it was omnipresent and was simple (so that even Web Designers without a Computer Science degree could use it). Now, there are vast differences between browsers and the toolchain has grown huge. We need to revisit the scripting in browsers. Instead of twisting arms to use "JavaScript", because of it "universality", we should focus on WebAssembly and the likes. Have a safe scripting VM in the browser, use any language that compiles to it - no need for Babel and the likes.
- dap 11y agoThis is a microbenchmark. It's a tight loop that calls the function 500 million times. The function itself just adds two numbers. It's pretty close to the best possible improvement for inlining a function. If that's what your program does, and it's a big part of what your program does, then inlining it may be a big performance win. Even then, addressing this may not be a worthwhile tradeoff. Even then, as others have pointed out, you could use a minimizer. If your program is not CPU-bound, or if it doesn't have a function that's called millions of times in a tight loop, or if that doesn't make up the majority of time spent on CPU, or if that function does more than execute a couple of instructions, then the performance difference will likely be enormously less.
- fla 11y agoI think the oddity here is that the function length takes the comments into account.
- dap 11y agoAgreed that's interesting (in a mostly academic sense), but the thrust of the post is about using that fact to make choices in writing code, and there's an awful lot of discussion here about that idea. That's what I was addressing.
- bru_ 11y agoAt first I thought the comment was actually serious and couldn't believe it before I read the whole article. Thanks for pointing this out
- mixonic 11y agoThis is a good thing to raise awareness of, but the solution is poor advice. Asking developers to remember the "gotcha" of 600 characters is not really viable. Instead, if this is important to you, consider the addition of minification or comment stripping to your production deployment process. Minification will also make the variable names and syntax use shorter, saving you further precious characters. Learn the problem, but automate the solution.
- nevi-me 11y agoIn the context of the article being on nodejs code; I don't know about other developers, but I don't minify my production nodejs code because it runs directly on the server. Whether I have `var nameThatIsStupidlyLong` or `var nTISL` doesn't make a difference because size of code isn't a concern. What do other developers out there do?
- smt88 11y agoYeah, but the whole point of the article is that the names of your variables (and even your comments!!) do make a difference in performance.
- nacs 11y agoIn this one context, this one micro-optimization applies but for 99% of other cases, they don't (and shouldn't). Note the 'benchmark' in OP is run 500 million times to see the performance difference. This is definitely not a common scenario.
- amelius 11y agoIsn't there a way to force the inline optimization of a function, regardless of the character count?
- mhd 11y agoOne comment mentions a quite significant speedup when switching the variable initialization step of the for loop from let to var. As excepted, you'll also get this if you keep the "let" keyword, but move it outside of the loop. What kind of optimization step is prevented here?
- vvoyer 11y agoWho need this? Prople writing really really performant code like parsers. Who write parsers? 1% of ppl? Sad this get so much exposure. Stop losing time with micro optimization, this is a bug that's it.
- vivram 11y agoGood to know as a "gotcha" but typically when you get to an optimization like this you should have covered all other bigger optimizations like minification, ... that this won't actually return big yields.
- bshimmin 11y agoThis cheerfully reminded me of the thing in MRI Ruby of (relatively recent) yesteryear where strings longer than 23 characters were twice as slow, qv. http://patshaughnessy.net/2012/1/4/never-create-ruby-strings-longer-than-23-characters http://patshaughnessy.net/2012/1/4/never-create-ruby-strings...
- 746F7475 11y agoI'm at work and can not test this out, but does adding whitespaces increase the function length here?