[WIP] lib: optimize util.format() - #215
bnoordhuis wants to merge 3 commits into
Conversation
Sorry, something went wrong.
Make a number of functions from require('_linklist') return the list or
the item just inserted. Makes the API a little less unwieldy to use.
Add a simple (too simple!) benchmark for util.format().
Optimize util.format() by parsing the format string and generating specialized code on the fly, then caching the result in a LRU list. This change is based on the observation that most applications that log extensively, often have very skewed distributions of log patterns. It's common to see top 10 or top 25 of popular patterns, followed by a long (sometimes very long) tail of less popular patterns. This is a work in progress: the common case is currently 2-25x faster but the worst case - every pattern unique - is 4-5x slower.
There was a problem hiding this comment.
Note to self: the generated code gets optimized as of https://codereview.chromium.org/821553003/
Sorry, something went wrong.
|
@bnoordhuis Side note, I've noticed that v8::Script::Compile() then v8::Script::Run() is 2x faster than new Function() from JS. |
Sorry, something went wrong.
|
I have benchmark results using template literals vs use "+" vs util.format. Just FYI: |
Sorry, something went wrong.
|
@yosuke-furukawa I'll have to refer you to http://mrale.ph/blog/2012/12/15/microbenchmarks-fairy-tale.html. The benchmark you show can be pretty much optimized out completely. |
Sorry, something went wrong.
There was a problem hiding this comment.
Isn’t /[\\"\r\n\u2028\u2029]/g enough?
Sorry, something went wrong.
There was a problem hiding this comment.
Now that you bring it up: probably not. With Harmony template strings being available now, the generated code is susceptible to template literal injection. I'll have to look at this again.
Sorry, something went wrong.
There was a problem hiding this comment.
/[\\"\r\n\u2028\u2029]/g is enough, and template strings don’t affect the parsing of double-quoted string literals.
Sorry, something went wrong.
|
Since this introduces a new module (sprintf) which conflicts with a similar module on npm (npm.im/sprintf), should this be a major version bump. The sprintf should also probably be documented. |
Sorry, something went wrong.
There was a problem hiding this comment.
Did you want to use an internal argc instead of arguments.length for for loop as a cached value? Otherwise argc is unused.
Sorry, something went wrong.
|
Since #848 landed, we can probably introduce sprintf as an internal module. |
Sorry, something went wrong.
|
Is anything happening with this at this point, or can it be closed? |
Sorry, something went wrong.
|
I'll close it and revisit when I have time. |
Sorry, something went wrong.
Optimize util.format() by parsing the format string and generating
specialized code on the fly, then caching the result in a LRU list.
This change is based on the observation that most applications that
log extensively, often have very skewed distributions of log patterns.
It's common to see top 10 or top 25 of popular patterns, followed by
a long (sometimes very long) tail of less popular patterns.
This is a work in progress: the common case is currently 2-25x faster
but the worst case - every pattern unique - is 4-5x slower.
WIP, soliciting comments.