added fix for utf8 multi-byte boundary when it is split between chunks, ...

[jlank]

...also created a test for this scenario, and made sure all current tests still pass in node and on the browser side. Sorry for the kinda messy diff, a force of habit of mine is removing trailing whitespace in vim with ,ss, it's so fun to get rid of them! :P

[jlank]

In reference to #11

[dscape]

@jlank are you using clarinet in production? would be great to have that switch replaced with if then elses, cause I know it's a problem for V8

cc @mraleph @felixge

[dscape]

Publishing this right now. Reference to the other case:

creationix/jsonparse#9

[felixge]

? would be great to have that switch replaced with if then elses, cause I know it's a problem for V8

I don't think it causes any problems, but it essentially performs the same as if / else would. That is: it goes through each case and evaluates it one after the other, rather than using some form of optimized lookup table to jump into the right case directly. My workaround this has always been to reduce the number of cases / times I have to hit the switch statement itself, which is usually accomplished by buffering more data and keeping more state of the state machine in my function scope. (Also: my understanding of this is probably very limited, so don't apply this advise unless @mraleph confirms this / you can verify it in a benchmark independently)

Related: Seems like @indutny tried to improve this before: https://codereview.chromium.org/8499010/

[mraleph]

If switch cases are integers it would not behave any different from a sequence of ifs.

If switch cases are string literals then in some cases it might behave worse: https://code.google.com/p/v8/issues/detail?id=2684

If switch fits neither of those two cases the function it is contained in is never optimized.

Manually reordering switch to have most common cases on top will improve performance. At least until V8 starts doing some sophisticated switch optimizations itself.

[jlank]

@dscape I'm not using it in production, I identified the bug initially using @dominictarr's JSONStream which depends on jsonparse and noticed this same issue was present for clarinet. Also I'm unclear on which switch statement you are referring to (I don't think my PR added one), I'd be happy to convert it if you could give me a line number though.

[ariya]

Echoing what @mraleph said above.

My own experience on the use of switch case: http://ariya.ofilabs.com/2012/04/javascript-switch-case-deoptimization.html. If you can run V8 and check your code, that gives a better insight than any amount of guessing :)

Profile-guided tweaks is also easy: http://ariya.ofilabs.com/2012/02/javascript-branching-and-code-shuffling.html.

Just my $0.02.

[indutny]

You might like this then: https://codereview.chromium.org/16128004/

[jlank]

@ariya thanks for the insightful blog posts, they give good context. I (think I) understand what @dscape was getting at now. I can go ahead and try an implementation with switch case's and see what benchmarks better in V8 instead of speculating.

[dscape]

@jlank that would be awesome!

[ariya]

@indutny Yep, looking forward to trying it out :)

[indutny]

@ariya It hasn't landed, but you can try pulling it in and running benchmarks to see if it helps.

[dscape]

Thank everyone for the input!! :) This is why open source is awesome.

@jlank would love to read a blog post about your findings in the end, sounds like you'll have a lot of insight into this by then 💯

[jlank]

@dscape you can count on it.. I have a draft post from my initial fix to jsonparse that is sitting in the publish queue. I'll use the benchmarking for clarinet to round out the post. stay tuned...