Skip to content

Commit

Permalink
buffer: reduce overhead of StringBytes::Encode for UCS2
Browse files Browse the repository at this point in the history
Currently calling StringBytes::Encode on a UCS2 buffer
results in two copies of the buffer and the usage of
std::vector::assign makes the memory usage unpredictable,
and therefore hard to test against.

This patch makes the memory usage more predictable by
allocating the memory using node::UncheckedMalloc and
handles the memory allocation failure properly. Only
one copy of the buffer will be created and it will
be freed upon GC of the string.

PR-URL: #19798
Refs: #19739
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
joyeecheung authored and targos committed Apr 12, 2018
1 parent ee63bfa commit 2209b81
Showing 1 changed file with 11 additions and 7 deletions.
18 changes: 11 additions & 7 deletions src/string_bytes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -775,15 +775,19 @@ MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
// Buffer, so we need to reorder on BE platforms. See
// http://nodejs.org/api/buffer.html regarding Node's "ucs2"
// encoding specification
std::vector<uint16_t> dst;
if (IsBigEndian()) {
dst.assign(buf, buf + buflen);
size_t nbytes = buflen * sizeof(dst[0]);
SwapBytes16(reinterpret_cast<char*>(&dst[0]), nbytes);
buf = &dst[0];
uint16_t* dst = node::UncheckedMalloc<uint16_t>(buflen);
if (dst == nullptr) {
*error = SB_MALLOC_FAILED_ERROR;
return MaybeLocal<Value>();
}
size_t nbytes = buflen * sizeof(uint16_t);
memcpy(dst, buf, nbytes);
SwapBytes16(reinterpret_cast<char*>(dst), nbytes);
return ExternTwoByteString::New(isolate, dst, buflen, error);
} else {
return ExternTwoByteString::NewFromCopy(isolate, buf, buflen, error);
}

return ExternTwoByteString::NewFromCopy(isolate, buf, buflen, error);
}

MaybeLocal<Value> StringBytes::Encode(Isolate* isolate,
Expand Down

0 comments on commit 2209b81

Please sign in to comment.