Skip to content

Commit 30c0921

Browse files
anonriganonrig
committed
address pr reviews
1 parent dc480e5 commit 30c0921

File tree

2 files changed

+34
-21
lines changed

2 files changed

+34
-21
lines changed

‎src/workerd/api/encoding.c++‎

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -466,52 +466,64 @@ jsg::Ref<TextEncoder> TextEncoder::constructor(jsg::Lock& js) {
466466
}
467467

468468
jsg::BufferSource TextEncoder::encode(jsg::Lock& js, jsg::Optional<jsg::JsString> input) {
469-
auto str = input.orDefault(js.str());
469+
jsg::JsString str = input.orDefault(js.str());
470470

471-
// Do the conversion while ValueView is alive, but to a C++ heap buffer (not V8 heap)
472-
kj::Array<kj::byte> output_data;
471+
if (str.length(js) == 0) {
472+
return jsg::BufferSource(js, jsg::BackingStore::alloc<v8::Uint8Array>(js, 0));
473+
}
473474

474-
{
475+
// Allocate buffer and write conversion results directly, using BackingStore::wrap to avoid
476+
// heap allocation restrictions while ValueView is alive and avoid copy overhead
477+
jsg::BackingStore backing = [&]() {
475478
v8::String::ValueView value_view(js.v8Isolate, str);
476479
size_t length = static_cast<size_t>(value_view.length());
477480

478481
if (value_view.is_one_byte()) {
479482
auto data = reinterpret_cast<const char*>(value_view.data8());
480483
size_t utf8_length = simdutf::utf8_length_from_latin1(data, length);
481-
output_data = kj::heapArray<kj::byte>(utf8_length);
484+
auto* output = new kj::Array<kj::byte>(kj::heapArray<kj::byte>(utf8_length));
482485
[[maybe_unused]] auto written =
483-
simdutf::convert_latin1_to_utf8(data, length, output_data.asChars().begin());
484-
KJ_DASSERT(written == output_data.size());
486+
simdutf::convert_latin1_to_utf8(data, length, output->asChars().begin());
487+
KJ_DASSERT(written == output->size());
488+
return jsg::BackingStore::wrap<v8::Uint8Array>(output->begin(), output->size(),
489+
[](void*, size_t, void* ptr) { delete reinterpret_cast<kj::Array<kj::byte>*>(ptr); },
490+
output);
485491
} else {
486492
auto data = reinterpret_cast<const char16_t*>(value_view.data16());
487493

488494
// Check if UTF-16LE is valid
489-
auto validation_result = simdutf::validate_utf16le(data, length);
495+
auto valid_utf16 = simdutf::validate_utf16le(data, length);
490496

491-
if (validation_result) {
497+
if (valid_utf16) {
492498
// Valid UTF-16LE, convert directly
493499
size_t utf8_length = simdutf::utf8_length_from_utf16le(data, length);
494-
output_data = kj::heapArray<kj::byte>(utf8_length);
500+
auto* output = new kj::Array<kj::byte>(kj::heapArray<kj::byte>(utf8_length));
495501
[[maybe_unused]] auto written =
496-
simdutf::convert_utf16le_to_utf8(data, length, output_data.asChars().begin());
497-
KJ_DASSERT(written == output_data.size());
502+
simdutf::convert_utf16le_to_utf8(data, length, output->asChars().begin());
503+
KJ_DASSERT(written == output->size());
504+
return jsg::BackingStore::wrap<v8::Uint8Array>(output->begin(), output->size(),
505+
[](void*, size_t, void* ptr) { delete reinterpret_cast<kj::Array<kj::byte>*>(ptr); },
506+
output);
498507
} else {
499508
// Invalid UTF-16LE (unpaired surrogates), fix it first
500509
auto well_formed = kj::heapArray<char16_t>(length);
501510
simdutf::to_well_formed_utf16le(data, length, well_formed.begin());
502511

503512
// Now convert the well-formed UTF-16LE to UTF-8
504513
size_t utf8_length = simdutf::utf8_length_from_utf16le(well_formed.begin(), length);
505-
output_data = kj::heapArray<kj::byte>(utf8_length);
514+
auto* output = new kj::Array<kj::byte>(kj::heapArray<kj::byte>(utf8_length));
506515
[[maybe_unused]] auto written = simdutf::convert_utf16le_to_utf8(
507-
well_formed.begin(), length, output_data.asChars().begin());
508-
KJ_DASSERT(written == output_data.size());
516+
well_formed.begin(), length, output->asChars().begin());
517+
KJ_DASSERT(written == output->size());
518+
return jsg::BackingStore::wrap<v8::Uint8Array>(output->begin(), output->size(),
519+
[](void*, size_t, void* ptr) { delete reinterpret_cast<kj::Array<kj::byte>*>(ptr); },
520+
output);
509521
}
510522
}
511-
} // ValueView destroyed here, releasing the heap lock
523+
}(); // ValueView destroyed here, releasing the heap lock
512524

513-
// Now create BufferSource from the output data (this allocates V8 objects, which is now safe)
514-
return jsg::BufferSource(js, jsg::BackingStore::from(js, kj::mv(output_data)));
525+
// Now create BufferSource from the BackingStore (this allocates V8 objects, which is now safe)
526+
return jsg::BufferSource(js, kj::mv(backing));
515527
}
516528

517529
TextEncoder::EncodeIntoResult TextEncoder::encodeInto(

‎src/workerd/jsg/buffersource.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,10 @@ class BackingStore {
102102

103103
// Creates a new BackingStore of the given size.
104104
template <BufferSourceType T = v8::Uint8Array>
105-
static BackingStore alloc(Lock& js, size_t size) {
106-
return BackingStore(js.allocBackingStore(size), size, 0, getBufferSourceElementSize<T>(),
107-
construct<T>, checkIsIntegerType<T>());
105+
static BackingStore alloc(
106+
Lock& js, size_t size, Lock::AllocOption init_mode = Lock::AllocOption::ZERO_INITIALIZED) {
107+
return BackingStore(js.allocBackingStore(size, init_mode), size, 0,
108+
getBufferSourceElementSize<T>(), construct<T>, checkIsIntegerType<T>());
108109
}
109110

110111
using Disposer = void(void*, size_t, void*);

0 commit comments

Comments
 (0)