buffer: port byteLengthUtf8 to JavaScript

[indutny]

Please don't land this PR yet. There is a missing V8 optimization, that is going to be available soon
in V8 6.6.

---

Prior to this change the majority of the time spent when
calling Buffer.byteLength was spent on crossing JS->C++ boundary. This
change move the function to JavaScript, making it much faster.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

Affected core subsystem(s)

buffer

[jasnell]

Performance boost I assume?

[indutny]

@jasnell forgot to write about it. It is 2x faster on small strings, but is slower on large ones. This is the reason for awaiting for v8 update.

[jasnell]

If the perf loss on the large ones is significant, perhaps split the difference and use the js method for short ones and fallback on longer?

[indutny]

Perhaps, but I'd rather wait for V8 update. We've been living with C++ code for awhile now.

[indutny]

Added expected V8 version.

[bnoordhuis]

len += 1; for consistency with line 348.

[indutny]

Nitpicking 😉

[indutny]

Looks like this might be ready after #19201 lands.

[indutny]

Hm... it is still slow even on V8 6.6. cc @bmeurer

Here's the benchmark that I used: https://gist.github.com/indutny/0fe9a3ed3b93d558a60deacefc37e378 . The results are:

c++: 7.052ms
js: 6.731ms
c++: 235.835ms
js: 5739.299ms

[bmeurer]

You need to pass --nountrusted-code-mitigations to disable the Spectre mitigations, which also affect String#charCodeAt. But even then the C++ code is going to be significantly faster. I'm not sure we can fix the difference here.

[indutny]

@bmeurer tbh, I don't see a reason for it to be slow. The performance seems to be dependent on string flattening, which apparently doesn't happen in JS case. Could it potentially happen at all?

[bmeurer]

@indutny The reason is that String#charCodeAt() has to dispatch on string type all the time (potentially 8 different types for the "fast case"), whereas the C++ code just accesses the character data. We could try to make TurboFan a bit smarter here if we find a way to do the flattening (which is indeed happening here) in the peeled loop iteration only and then have only sequential one-byte string access inside the actual loop. But that's work that needs to be done and I'm a bit worried that we over-optimize for the particular example.

[bmeurer]

BTW V8 tip-of-tree should be faster already thanks to the recent work by @sigurdschneider.

[indutny]

@bmeurer thank you!

@jasnell and @nodejs/collaborators looks like it'd best to close this.

[bmeurer]

Not saying it's not worth it, but it's definitely not high priority.

Did you check with tip-of-tree? And --nountrusted-code-mitigations?

[indutny]

@bmeurer I checked --nountrusted-code-mitigations, and although it was faster - it was still significantly slower than C++. Didn't check tip-of-tree yet.

[srl295]

seems ok (did not check the math in detail)…  might also compare to http://source.icu-project.org/repos/icu/trunk/icu4c/source/common/unicode/utf8.h which has been highly optimized

[BridgeAR]

Should this stay open? It seems like the JS implementation can not get as fast as the C++ implementation. Ping @indutny

[BridgeAR]

Closing as this does not seem like it is the right approach in JS.

@indutny please reopen in case you disagree and want to continue working on this.

[indutny]

I think we could potentially apply it to short strings. Anyone up to do some benchmarks?