Closed brianc closed 12 years ago
You should use Buffer('\0', 'binary')
instead of Buffer('\0')
with Node.js v0.3.x, because default encoding is 'utf8' now.
In v0.2.x this worked: Buffer('!\0', 'utf8') => <Buffer 33 00>
and now same thing results in <Buffer 33>
I stumbled over this, too. It feels like an arbitrary workaround for a string terminator problem somewhere else. Buffer.byteLength('\u0000') actually does return 1 but new Buffer('\u0000') cuts off a trailing null byte: https://github.com/ry/node/blob/master/src/node_buffer.cc#L430
'binary' encoding works, but "this encoding method is depreciated and should be avoided in favor of Buffer objects where possible. This encoding will be removed in future versions of Node." (http://nodejs.org/docs/v0.3.2/api/buffers.html#buffers)
This is still a problem. In v0.4.7, new Buffer('\0', 'ascii')
results in <Buffer 20>
.
It's a bug / feature in v8::String::WriteAscii() that translates nul bytes into spaces[1]. V8 has always done that but Node didn't trigger that code path in 0.2.x.
[1] https://github.com/v8/v8/blob/e3319f4/src/api.cc#L3615 (warning, big file)
The problem is that V8 appends a nul byte if there is still room left in the output buffer. We can work around that by passing in the exact byte length as returned by v8::String::Utf8Length()
. It's slower but correct (and not even that much slower). Ad-hoc benchmark:
buf = Buffer(64);
for (var i = 0; i < 10e6; ++i) {
buf.write('ö日本語abcdefäëö', 0, 'utf8');
}
Current master:
$ time ./node tmp/utf8.js
real 0m9.346s
user 0m9.350s
sys 0m0.050s
With 22fabd4 applied:
$ time ./node tmp/utf8.js
real 0m11.213s
user 0m11.230s
sys 0m0.020s
An alternative take is to patch V8 to not append '\0' if requested. That might be necessary for #297 anyway (0x00 is converted to 0x20 in ASCII input) because there is no way right now to side-step that.
@ry Can you review?
with 6079f1a applied?
I have tested using long long string when I wrote my first patch (e50a751), it was twice slower if I called v8::String::Utf8Length()
in my environment (v0.4/V8 3.1).
long string version:
var a = [];
for (var i = 0; i < 2000; ++i) {
a.push('蛯原友里'); // 3bytes * 4
}
var s = a.join('');
var buf = new Buffer(24000);
for (i = 0; i < 10e4; ++i) {
buf.write(s, 0, 'utf8');
}
current master:
real 0m9.739s
user 0m10.660s
sys 0m4.480s
with 22fabd4 applied:
real 0m12.709s
user 0m14.390s
sys 0m0.970s
with 6079f1a applied:
real 0m10.170s
user 0m10.580s
sys 0m5.110s
I can't wait HINT_NO_NULL_TERMINATOR
!
http://code.google.com/p/v8/issues/detail?id=1537
I see a similar ~40% performance drop with Utf8Length()
on long strings so that's out the window. But having a workaround for a workaround doesn't strike me as a great idea either, no offense to your fine work, koichik.
I suppose we should lobby the V8 guys to fix it but I suspect the patch from that issue you linked to won't pass muster.
Never mind, I agree Ben. My patch is workaround to avoid calling Utf8Lenght()
.
But many Web application especially using template engine such as EJS makes long string.
So we need this workaround I think.
May I tag "v8" on this issue? :-)
@koichik Go for it. :)
In node v0.3.0 buffers are not encoding null character '\0' correctly
Totally breaks creation of null terminated strings within buffers