Skip to content

Commit df40e48

Browse files
committed
fix: keep the encodeInto fast path off the JS heap
Mirrors ios#448 c29e30fd — and matters more here, where the JIT makes the fast overload live. WriteUtf8V2 flattens a cons string, and flattening allocates on the JS heap, which a fast callback must never do. The fast overload now takes the source as kSeqOneByteString, so V8 routes cons and two-byte strings to the slow callback by construction, and the flat latin-1 units it does receive are encoded by hand with no V8 string calls at all. Materializing an on-heap typed array's buffer allocates too, so the op now returns a status code: the fast path declines such views with kEncodeIntoRetrySlow and the builtin finishes through encodeIntoFallback. The {read, written} array moves to the binding, built on a native ArrayBuffer, which is off-heap from birth. Also addresses this PR's review round: the docs now attribute the label table to TextDecoder (TextEncoder is UTF-8-only per spec), and the encoding-order worker spec reports worker errors instead of timing out.
1 parent 01bc3bd commit df40e48

4 files changed

Lines changed: 118 additions & 30 deletions

File tree

docs/text-encoding.md

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,12 @@ Node's split: `js/text-encoding.js` owns the WebIDL surface (brand checks via
3333
private fields, enumerable prototype members, `Symbol.toStringTag`),
3434
`TextEncoding.cpp` owns the bytes.
3535

36-
- **Encodings**: utf-8, utf-16le, utf-16be and windows-1252, each with its
37-
complete WHATWG label set; an unknown label throws `RangeError`. (Precedent:
38-
Node without ICU ships utf-8/utf-16le; utf-16be and windows-1252 are cheap,
39-
and windows-1252 covers the `ascii`/`latin1`/`iso-8859-1` aliases web code
40-
actually uses.)
36+
- **Decoder encodings**: the `TextDecoder` constructor resolves utf-8,
37+
utf-16le, utf-16be and windows-1252, each with its complete WHATWG label
38+
set; an unknown label throws `RangeError`. (Precedent: Node without ICU
39+
ships utf-8/utf-16le; utf-16be and windows-1252 are cheap, and windows-1252
40+
covers the `ascii`/`latin1`/`iso-8859-1` aliases web code actually uses.)
41+
`TextEncoder` is UTF-8-only and takes no label, as the spec defines it.
4142
- **Streaming**: full `decode(…, { stream: true })` support. Incomplete
4243
sequences (split BOMs and split utf-16 code units included) carry across
4344
calls in a 16-byte `Uint8Array` the builtin owns — no per-instance native

test-app/app/src/main/assets/app/tests/testNsUtil.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@ describe("ns:util", function () {
4646
done();
4747
}
4848
};
49+
worker.onerror = function (error) {
50+
fail("worker (" + order + ") failed: " + error.message);
51+
worker.terminate();
52+
done();
53+
};
4954
worker.postMessage(order);
5055
});
5156
});

test-app/runtime/src/main/cpp/TextEncoding.cpp

Lines changed: 88 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -580,24 +580,32 @@ void EncodeUtf8Callback(const FunctionCallbackInfo<Value>& info) {
580580
info.GetReturnValue().Set(Uint8Array::New(buffer, 0, length));
581581
}
582582

583+
// encodeInto status codes, mirrored in text-encoding.js.
584+
constexpr int32_t kEncodeIntoOk = 0;
585+
constexpr int32_t kEncodeIntoBadDestination = 1;
586+
// Fast path only: the view's buffer is still on the V8 heap and Buffer() would
587+
// allocate to materialize it, which a fast callback must not do. The builtin
588+
// retries through encodeIntoFallback, which always runs the slow callback.
589+
constexpr int32_t kEncodeIntoRetrySlow = 2;
590+
583591
// Writes as much of `source` as fits into `destination` without splitting an
584-
// encoded code point, and reports {read, written} through `results` — a
585-
// Uint32Array the builtin owns, so the op returns only the destination
586-
// type check and stays expressible as a fast call. A destination that is a
587-
// Uint8Array but detached or empty is a zero-length write, not a failure.
588-
bool EncodeIntoImpl(Isolate* isolate, Local<Value> sourceValue,
589-
Local<Value> destinationValue, Local<Value> resultsValue) {
592+
// encoded code point, and reports {read, written} through `results` — the
593+
// Uint32Array the binding owns, so the op returns only a status code and
594+
// stays expressible as a fast call. A destination that is a Uint8Array but
595+
// detached or empty is a zero-length write, not a failure.
596+
int32_t EncodeIntoImpl(Isolate* isolate, Local<Value> sourceValue,
597+
Local<Value> destinationValue, Local<Value> resultsValue) {
590598
if (!destinationValue->IsUint8Array()) {
591-
return false;
599+
return kEncodeIntoBadDestination;
592600
}
593601
if (!sourceValue->IsString() || !resultsValue->IsUint32Array()) {
594-
return true;
602+
return kEncodeIntoOk;
595603
}
596604

597605
Local<Uint32Array> results = resultsValue.As<Uint32Array>();
598606
uint32_t* resultData = static_cast<uint32_t*>(results->Buffer()->Data());
599607
if (resultData == nullptr || results->Length() < 2) {
600-
return true;
608+
return kEncodeIntoOk;
601609
}
602610
resultData += results->ByteOffset() / sizeof(uint32_t);
603611
resultData[0] = 0;
@@ -607,7 +615,7 @@ bool EncodeIntoImpl(Isolate* isolate, Local<Value> sourceValue,
607615
void* base = destination->Buffer()->Data();
608616
const size_t capacity = destination->ByteLength();
609617
if (base == nullptr || capacity == 0) {
610-
return true;
618+
return kEncodeIntoOk;
611619
}
612620

613621
size_t read = 0;
@@ -616,7 +624,7 @@ bool EncodeIntoImpl(Isolate* isolate, Local<Value> sourceValue,
616624
capacity, v8::String::WriteFlags::kReplaceInvalidUtf8, &read);
617625
resultData[0] = static_cast<uint32_t>(read);
618626
resultData[1] = static_cast<uint32_t>(written);
619-
return true;
627+
return kEncodeIntoOk;
620628
}
621629

622630
void EncodeIntoCallback(const FunctionCallbackInfo<Value>& info) {
@@ -625,14 +633,64 @@ void EncodeIntoCallback(const FunctionCallbackInfo<Value>& info) {
625633
}
626634

627635
#if NATIVESCRIPT_ENABLE_FAST_API
628-
// Fast-call overload of encodeInto. It allocates nothing on the V8 heap and
629-
// calls no JS.
630-
bool FastEncodeInto(Local<Value> receiver, Local<Value> source,
631-
Local<Value> destination, Local<Value> results,
632-
// NOLINTNEXTLINE(runtime/references)
633-
FastApiCallbackOptions& options) {
636+
// Fast-call overload of encodeInto, live once a call site tiers up. A fast
637+
// callback must not allocate on the JS heap, which shapes all three inputs:
638+
// the kSeqOneByteString parameter keeps cons and two-byte sources on the slow
639+
// callback (WriteUtf8V2 flattens, which allocates) and the latin-1 units are
640+
// encoded by hand; a view whose buffer is still on-heap is declined with
641+
// kEncodeIntoRetrySlow rather than materialized.
642+
int32_t FastEncodeInto(Local<Value> receiver, const FastOneByteString& source,
643+
Local<Value> destinationValue, Local<Value> resultsValue,
644+
// NOLINTNEXTLINE(runtime/references)
645+
FastApiCallbackOptions& options) {
634646
HandleScope scope(options.isolate);
635-
return EncodeIntoImpl(options.isolate, source, destination, results);
647+
if (!destinationValue->IsUint8Array()) {
648+
return kEncodeIntoBadDestination;
649+
}
650+
if (!resultsValue->IsUint32Array()) {
651+
return kEncodeIntoOk;
652+
}
653+
Local<Uint8Array> destination = destinationValue.As<Uint8Array>();
654+
Local<Uint32Array> results = resultsValue.As<Uint32Array>();
655+
if (!destination->HasBuffer() || !results->HasBuffer()) {
656+
return kEncodeIntoRetrySlow;
657+
}
658+
659+
uint32_t* resultData = static_cast<uint32_t*>(results->Buffer()->Data());
660+
if (resultData == nullptr || results->Length() < 2) {
661+
return kEncodeIntoOk;
662+
}
663+
resultData += results->ByteOffset() / sizeof(uint32_t);
664+
resultData[0] = 0;
665+
resultData[1] = 0;
666+
667+
void* base = destination->Buffer()->Data();
668+
const size_t capacity = destination->ByteLength();
669+
if (base == nullptr || capacity == 0) {
670+
return kEncodeIntoOk;
671+
}
672+
uint8_t* out = static_cast<uint8_t*>(base) + destination->ByteOffset();
673+
674+
size_t read = 0;
675+
size_t written = 0;
676+
for (; read < source.length; read++) {
677+
const uint8_t unit = static_cast<uint8_t>(source.data[read]);
678+
if (unit < 0x80) {
679+
if (written + 1 > capacity) {
680+
break;
681+
}
682+
out[written++] = unit;
683+
} else {
684+
if (written + 2 > capacity) {
685+
break;
686+
}
687+
out[written++] = 0xC0 | (unit >> 6);
688+
out[written++] = 0x80 | (unit & 0x3F);
689+
}
690+
}
691+
resultData[0] = static_cast<uint32_t>(read);
692+
resultData[1] = static_cast<uint32_t>(written);
693+
return kEncodeIntoOk;
636694
}
637695

638696
const CFunction kFastEncodeInto = CFunction::Make(FastEncodeInto);
@@ -653,6 +711,18 @@ MaybeLocal<Object> CreateBinding(Local<Context> context) {
653711
#else
654712
tns::SetMethod(context, binding, "encodeInto", EncodeIntoCallback);
655713
#endif
714+
// Same slow callback with no fast overload: where the fast path answers
715+
// kEncodeIntoRetrySlow, the builtin finishes the call through this name.
716+
tns::SetMethod(context, binding, "encodeIntoFallback", EncodeIntoCallback);
717+
718+
// Native ArrayBuffers carry a real backing store from birth, so the fast
719+
// path's HasBuffer test always passes for the results array.
720+
Local<ArrayBuffer> resultsBuffer = ArrayBuffer::New(isolate, 2 * sizeof(uint32_t));
721+
if (!binding->Set(context, ArgConverter::ConvertToV8String(isolate, "encodeIntoResults"),
722+
Uint32Array::New(resultsBuffer, 0, 2))
723+
.FromMaybe(false)) {
724+
return MaybeLocal<Object>();
725+
}
656726

657727
return binding;
658728
}

test-app/runtime/src/main/cpp/js/text-encoding.js

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,16 @@ const {
1919
SymbolToStringTag,
2020
TypeError,
2121
Uint8Array,
22-
Uint32Array,
2322
} = primordials;
2423

25-
const { labelToEncoding, decode, encodeUtf8, encodeInto } = binding;
24+
const {
25+
labelToEncoding,
26+
decode,
27+
encodeUtf8,
28+
encodeInto,
29+
encodeIntoFallback,
30+
encodeIntoResults,
31+
} = binding;
2632

2733
// Indexed by the encoding ids labelToEncoding returns.
2834
const kEncodingNames = ["utf-8", "utf-16le", "utf-16be", "windows-1252"];
@@ -35,10 +41,12 @@ const kFlagStream = 4;
3541
// Mirrors TextEncoding::kDecoderStateSize.
3642
const kDecoderStateSize = 16;
3743

38-
// encodeInto reports {read, written} through this rather than allocating a
39-
// result object natively; the op is synchronous, so one buffer serves every
40-
// encoder in the isolate.
41-
const encodeIntoResults = new Uint32Array(2);
44+
// Mirror the kEncodeInto* status codes in TextEncoding.cpp. The op reports
45+
// {read, written} through binding.encodeIntoResults rather than allocating a
46+
// result object per call; it is synchronous, so that one native Uint32Array
47+
// serves every encoder in the isolate.
48+
const kEncodeIntoBadDestination = 1;
49+
const kEncodeIntoRetrySlow = 2;
4250

4351
// WebIDL dictionary conversion: undefined and null mean "all defaults",
4452
// anything else must be an object.
@@ -74,7 +82,11 @@ class TextEncoder {
7482
encodeInto(source, destination) {
7583
TextEncoder.#check(this);
7684
const text = `${source}`;
77-
if (!encodeInto(text, destination, encodeIntoResults)) {
85+
let code = encodeInto(text, destination, encodeIntoResults);
86+
if (code === kEncodeIntoRetrySlow) {
87+
code = encodeIntoFallback(text, destination, encodeIntoResults);
88+
}
89+
if (code === kEncodeIntoBadDestination) {
7890
throw new TypeError(
7991
'The "destination" argument must be an instance of Uint8Array'
8092
);

0 commit comments

Comments
 (0)