Parser performance boost #573

3rd-Eden · 2011-10-12T19:49:43Z

http://cl.ly/402t2C133B2a1P0g1K0h <--- before http://cl.ly/363W080c2j261l3A1m3y <--- after ;o

…oder

3rd-Eden · 2011-10-12T20:10:33Z

Including loop optimizations: http://cl.ly/0Y2f2J3Y1E311t3Z3C1C

ThisIsMissEm · 2011-10-12T20:14:03Z

lib/parser.js

@@ -141,14 +144,15 @@ exports.encodePayload = function (packets) {
 var regexp = /([^:]+):([0-9]+)?(\+)?:([^:]+)?:?([\s\S]*)?/;

 exports.decodePacket = function (data) {
-  var pieces = data.match(regexp);
+  var pieces = data.match(regexp)
+    , parse = JSON.parse;


Surely a micro-optimisation?

I saw a small increase of decoding per sec because of it.

ThisIsMissEm · 2011-10-12T20:15:08Z

For all the cases where you have defaults that are || '', could you perhaps cache that empty string value?

tj · 2011-10-12T20:21:45Z

I just wanted to play with vbench haha. we should find out if it's even remotely a bottleneck first, but the changes are not ugly or anything so it doesn't hurt

3rd-Eden · 2011-10-12T20:41:15Z

@miksago changing the switch statement to a integer switch and changing the empty string both degraded performance. It costed about 1K ops/s. But thanks a lot for the tips!

@visionmedia :p The protocol parser is a hot code path, so If we can optimize it, we should. For some applications these changes might not be noticeable because they are not processing a lot of message per sec, but for busy chat boxes or what ever it could certainly improve performance and response times.

tj · 2011-10-12T20:56:37Z

for sure, that's why I wanted to make sure there was no glaring issues with those first, but we should still profile the system as a whole

3rd-Eden · 2011-10-12T21:05:35Z

Not only that but also run individual parts against the v8 --profile option to see if we can optimize against crankshaft. Just to be sure that we are not causing it to bail out.

Because I'm sure we do that with the parser, because we wrapping JSON.parse in a try catch block.

ThisIsMissEm · 2011-10-12T21:14:55Z

@3rd-Eden Aha! That'd be an explaination as to why memoizing JSON.parse speeds up, as it doesn't need to look up that value every time due to try {} catch (e){} breaking crankshaft optimisations.

ThisIsMissEm · 2011-10-12T21:15:45Z

@3rd-Eden switch on integers is slower, but if/else on integers may be faster. I'm surprised caching '' is slower.

rauchg · 2011-10-12T22:10:02Z

niiiice

Parser performance boost

einaros · 2011-10-13T07:52:41Z

@3rd-Eden switch on integers is slower, but if/else on integers may be faster. I'm surprised caching '' is slower.

Not really integers in javascript, but anyway. That switches are marginally faster than if/else is no surprise, but the fact that string switches are quicker than number switches -- if true -- surely must be a javascript artifact.

einaros · 2011-10-13T08:26:07Z

I did a few simulations on pregenerated sequences of strings / numbers, with switch / ifelse / object-function-lookup for each possible value:

switch and object benchmarks: 
43070 x one of 10 possible 10 letter strings, in a pre-generated random order,
with blocks matching each possible string,
(a) run through a switch, (b) called as methods on an object.

timing shows milliseconds elapsed for 1000 repetitions of the 43070 long sequence.

v0.4.10
switchbench.js 2856ms
objectbench.js 1600ms

v0.5.9
switchbench.js 1879ms
objectbench.js 1319ms

Result: method name lookup on an object is faster.

===

number benchmarks: 
43070 numbers ranging from 0 to 9 inclusive, in a pre-generated random order, 
with blocks matching each possible number,
(a) run through an if/else block matching all 10 possibilities, (b) run through a switch.

timing shows milliseconds elapsed for 1000 repetitions of the 43070 long sequence.

v0.4.10
num-ifelse.js  1636ms
num-switch.js  1318ms

v0.5.9
num-ifelse.js  718ms
num-switch.js  595ms

Result: switch is marginally faster

Combined results: numeric matching should be faster than string matching (as expected)

einaros · 2011-10-13T08:29:24Z

If this holds true, even without doing more testing with numeric switches, rewriting the switch cases in the code affected by this pull to objects with functions should yield even quicker runtimes.

ThisIsMissEm · 2011-10-13T10:00:56Z

I think you may be doing those benchmarks in a slightly wrong way, you should be
trying to do as many calls as possible in a set time frame (as does benchmark.js).

On 13 Oct 2011, at 09:26, Einar Otto Stangvik wrote:

I did a few simulations on pregenerated sequences of strings / numbers, with switch / ifelse / object-function-lookup for each possible value:

switch and object benchmarks:
43070 x one of 10 possible 10 letter strings, in a pre-generated random order,
with blocks matching each possible string,
(a) run through a switch, (b) called as methods on an object.

timing shows milliseconds elapsed for 1000 repetitions of the 43070 long sequence.

v0.4.10
switchbench.js 2856ms
objectbench.js 1600ms

v0.5.9
switchbench.js 1879ms
objectbench.js 1319ms

Result: method name lookup on an object is faster.

===

number benchmarks:
43070 numbers ranging from 0 to 9 inclusive, in a pre-generated random order,
with blocks matching each possible number,
(a) run through an if/else block matching all 10 possibilities, (b) run through a switch.

timing shows milliseconds elapsed for 1000 repetitions of the 43070 long sequence.

v0.4.10
num-ifelse.js 1636ms
num-switch.js 1318ms

v0.5.9
num-ifelse.js 718ms
num-switch.js 595ms

Result: switch is marginally faster

Combined results: numeric matching should be faster than string matching (as expected)

Reply to this email directly or view it on GitHub:
#573 (comment)

3rd-Eden · 2011-10-13T10:09:22Z

I decided to port the current parser benchmark that TJ wrote in vbench to benchmark.js;

https://gist.github.com/1283881

So we can actually do some testing on node 0.5.9

einaros · 2011-10-13T10:28:10Z

@miksago, Uhm and how do you figure that makes any difference at all? So long as the sequences are equally long, the data doesn't lie.

ThisIsMissEm · 2011-10-13T10:31:11Z

@einaros I can't remember all the reasons, but I had a really long discussion with @jdalton about this at JSConf.eu, perhaps he can fill you in on the fine points there?

ThisIsMissEm · 2011-10-13T10:31:39Z

@3rd-Eden Nice! I spent about 6 hours trying to get npm to install on 0.5.10-pre :/

einaros · 2011-10-13T10:33:30Z

@miksago, Well benchmarking differently can paint a more realistic picture in cases where the processing speed increases / decreases over time. Here, however, that's not the case at all. All we really want to see is roughly how much time a single call through if / switch branching takes, vs. e.g. object member lookup.

tj · 2011-10-13T11:34:49Z

@3rd-Eden nice! benchmark.js works for you? it failed miserably last time I tried it so I went with uubench for vbench but if that's working properly we should change vbench to use it

jdalton · 2011-10-13T13:01:32Z

@visionmedia benchmark.js is a too common name :D
The benchmark.js @3rd-Eden is talking about is http://benchmarkjs.com (it powers jsPerf).

tj · 2011-10-13T13:04:58Z

oh, lame. I thought they had that stuff running on node too, but last time i looked it was a huge mess

3rd-Eden · 2011-10-13T13:05:25Z

@visionmedia I have used it couple of times before to benchmark some of my other modules and it worked quite nicely. I have no doubt about the quality of the code as it also powers jspref.

There are some small issues in the code, if you want to run multiple suites. You need to manually clear the cycle event from the last benchmark, or that one will be used.. Which is quite annoying :p

3rd-Eden · 2011-10-13T13:06:32Z

Doh, you guys type way faster than i do ;)

jdalton · 2011-10-13T13:32:57Z

You need to manually clear the cycle event from the last benchmark, or that one will be used.. Which is quite annoying :p

Lemmi know if that's still an issue. The npm module is totally out of date, I will try to push an update to it today.

tj · 2011-10-13T14:13:18Z

looked at it again, looks ok actually, I'd love to try it out for vbench

einaros · 2011-10-13T18:23:53Z

https://gist.github.com/48bf9fbcdb6993605fee <= Did a quick and dirty test with object method lookup rather than switch. Seems to provide slightly better speeds. More tests will follow.

einaros · 2011-10-13T18:35:01Z

Using arrays of functions + indexes rather than string types => way slower than all prior approaches. I'm still guessing this is due to javascript numbers being floating point rather than plain ints.

3rd-Eden · 2011-10-13T19:57:06Z

The speed performance isn't coming from changing the switch to a object method lookup, but from moving the try {} catch (e) {} related code blocks to another function. This way V8's crankshaft can fully optimize the rest of the functions.

A gist to illustrate: https://gist.github.com/1285245 =p

3rd-Eden · 2011-10-13T19:58:05Z

@jdalton Great to hear that, can't wait to see it land in npm :)

3rd-Eden added 3 commits October 12, 2011 21:44

http://cl.ly/402t2C133B2a1P0g1K0h <--- before http://cl.ly/363W080c2j…

709c172

…261l3A1m3y <--- after ;o

Optimized the loop, so the most commen packets are checked first

b662f2e

Returned the switch for the decoder, optimized the switch for the enc…

7800003

…oder

ThisIsMissEm reviewed Oct 12, 2011
View reviewed changes

rauchg added a commit that referenced this pull request Oct 12, 2011

Merge pull request #573 from 3rd-Eden/performance

373c729

Parser performance boost

rauchg merged commit 373c729 into socketio:master Oct 12, 2011

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parser performance boost #573

Parser performance boost #573

3rd-Eden commented Oct 12, 2011

3rd-Eden commented Oct 12, 2011

ThisIsMissEm Oct 12, 2011

3rd-Eden Oct 12, 2011

ThisIsMissEm commented Oct 12, 2011

tj commented Oct 12, 2011

3rd-Eden commented Oct 12, 2011

tj commented Oct 12, 2011

3rd-Eden commented Oct 12, 2011

ThisIsMissEm commented Oct 12, 2011

ThisIsMissEm commented Oct 12, 2011

rauchg commented Oct 12, 2011

einaros commented Oct 13, 2011

einaros commented Oct 13, 2011

einaros commented Oct 13, 2011

ThisIsMissEm commented Oct 13, 2011

3rd-Eden commented Oct 13, 2011

einaros commented Oct 13, 2011

ThisIsMissEm commented Oct 13, 2011

ThisIsMissEm commented Oct 13, 2011

einaros commented Oct 13, 2011

tj commented Oct 13, 2011

jdalton commented Oct 13, 2011

tj commented Oct 13, 2011

3rd-Eden commented Oct 13, 2011

3rd-Eden commented Oct 13, 2011

jdalton commented Oct 13, 2011

tj commented Oct 13, 2011

einaros commented Oct 13, 2011

einaros commented Oct 13, 2011

3rd-Eden commented Oct 13, 2011

3rd-Eden commented Oct 13, 2011

Parser performance boost #573

Parser performance boost #573

Conversation

3rd-Eden commented Oct 12, 2011

3rd-Eden commented Oct 12, 2011

ThisIsMissEm Oct 12, 2011

Choose a reason for hiding this comment

3rd-Eden Oct 12, 2011

Choose a reason for hiding this comment

ThisIsMissEm commented Oct 12, 2011

tj commented Oct 12, 2011

3rd-Eden commented Oct 12, 2011

tj commented Oct 12, 2011

3rd-Eden commented Oct 12, 2011

ThisIsMissEm commented Oct 12, 2011

ThisIsMissEm commented Oct 12, 2011

rauchg commented Oct 12, 2011

einaros commented Oct 13, 2011

einaros commented Oct 13, 2011

einaros commented Oct 13, 2011

ThisIsMissEm commented Oct 13, 2011

3rd-Eden commented Oct 13, 2011

einaros commented Oct 13, 2011

ThisIsMissEm commented Oct 13, 2011

ThisIsMissEm commented Oct 13, 2011

einaros commented Oct 13, 2011

tj commented Oct 13, 2011

jdalton commented Oct 13, 2011

tj commented Oct 13, 2011

3rd-Eden commented Oct 13, 2011

3rd-Eden commented Oct 13, 2011

jdalton commented Oct 13, 2011

tj commented Oct 13, 2011

einaros commented Oct 13, 2011

einaros commented Oct 13, 2011

3rd-Eden commented Oct 13, 2011

3rd-Eden commented Oct 13, 2011