Skip to content

Commit

Permalink
src: use AliasedUint32Array for encodeInto results
Browse files Browse the repository at this point in the history
Getting the buffer from a TypedArray created from the JS land
incurs a copy. For encodeInto() results we can just use an
AliasedArray and let the binding always own the store.

PR-URL: #46658
Reviewed-By: Darshan Sen <raisinten@gmail.com>
  • Loading branch information
joyeecheung authored and targos committed Mar 14, 2023
1 parent dcba3a0 commit 6b60f38
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 26 deletions.
15 changes: 8 additions & 7 deletions lib/internal/encoding.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ const {
StringPrototypeSlice,
Symbol,
SymbolToStringTag,
Uint32Array,
Uint8Array,
} = primordials;

Expand Down Expand Up @@ -49,12 +48,12 @@ const {
validateString,
validateObject,
} = require('internal/validators');

const binding = internalBinding('encoding_binding');
const {
encodeInto,
encodeUtf8String,
decodeUTF8,
} = internalBinding('encoding_binding');
} = binding;

const { Buffer } = require('buffer');

Expand Down Expand Up @@ -318,8 +317,6 @@ function getEncodingFromLabel(label) {
return encodings.get(trimAsciiWhitespace(label.toLowerCase()));
}

const encodeIntoResults = new Uint32Array(2);

class TextEncoder {
constructor() {
this[kEncoder] = true;
Expand All @@ -340,8 +337,12 @@ class TextEncoder {
validateString(src, 'src');
if (!dest || !isUint8Array(dest))
throw new ERR_INVALID_ARG_TYPE('dest', 'Uint8Array', dest);
encodeInto(src, dest, encodeIntoResults);
return { read: encodeIntoResults[0], written: encodeIntoResults[1] };

encodeInto(src, dest);
// We need to read from the binding here since the buffer gets refreshed
// from the snapshot.
const { 0: read, 1: written } = binding.encodeIntoResults;
return { read, written };
}

[inspect](depth, opts) {
Expand Down
44 changes: 27 additions & 17 deletions src/encoding_binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,30 @@ using v8::Local;
using v8::MaybeLocal;
using v8::Object;
using v8::String;
using v8::Uint8Array;
using v8::Uint32Array;
using v8::Uint8Array;
using v8::Value;

BindingData::BindingData(Environment* env, Local<Object> object)
: SnapshotableObject(env, object, type_int) {}
void BindingData::MemoryInfo(MemoryTracker* tracker) const {
tracker->TrackField("encode_into_results_buffer",
encode_into_results_buffer_);
}

BindingData::BindingData(Realm* realm, v8::Local<v8::Object> object)
: SnapshotableObject(realm, object, type_int),
encode_into_results_buffer_(realm->isolate(), kEncodeIntoResultsLength) {
object
->Set(realm->context(),
FIXED_ONE_BYTE_STRING(realm->isolate(), "encodeIntoResults"),
encode_into_results_buffer_.GetJSArray())
.Check();
}

bool BindingData::PrepareForSerialization(Local<Context> context,
v8::SnapshotCreator* creator) {
// We'll just re-initialize the buffers in the constructor since their
// contents can be thrown away once consumed in the previous call.
encode_into_results_buffer_.Release();
// Return true because we need to maintain the reference to the binding from
// JS land.
return true;
Expand All @@ -47,19 +62,19 @@ void BindingData::Deserialize(Local<Context> context,
InternalFieldInfoBase* info) {
DCHECK_EQ(index, BaseObject::kEmbedderType);
v8::HandleScope scope(context->GetIsolate());
Environment* env = Environment::GetCurrent(context);
Realm* realm = Realm::GetCurrent(context);
// Recreate the buffer in the constructor.
BindingData* binding = env->AddBindingData<BindingData>(context, holder);
BindingData* binding = realm->AddBindingData<BindingData>(context, holder);
CHECK_NOT_NULL(binding);
}

void BindingData::EncodeInto(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Isolate* isolate = env->isolate();
CHECK_GE(args.Length(), 3);
CHECK_GE(args.Length(), 2);
CHECK(args[0]->IsString());
CHECK(args[1]->IsUint8Array());
CHECK(args[2]->IsUint32Array());
BindingData* binding_data = Realm::GetBindingData<BindingData>(args);

Local<String> source = args[0].As<String>();

Expand All @@ -68,21 +83,16 @@ void BindingData::EncodeInto(const FunctionCallbackInfo<Value>& args) {
char* write_result = static_cast<char*>(buf->Data()) + dest->ByteOffset();
size_t dest_length = dest->ByteLength();

// results = [ read, written ]
Local<Uint32Array> result_arr = args[2].As<Uint32Array>();
uint32_t* results = reinterpret_cast<uint32_t*>(
static_cast<char*>(result_arr->Buffer()->Data()) +
result_arr->ByteOffset());

int nchars;
int written = source->WriteUtf8(
isolate,
write_result,
dest_length,
&nchars,
String::NO_NULL_TERMINATION | String::REPLACE_INVALID_UTF8);
results[0] = nchars;
results[1] = written;

binding_data->encode_into_results_buffer_[0] = nchars;
binding_data->encode_into_results_buffer_[1] = written;
}

// Encode a single string to a UTF-8 Uint8Array (not Buffer).
Expand Down Expand Up @@ -175,9 +185,9 @@ void BindingData::Initialize(Local<Object> target,
Local<Value> unused,
Local<Context> context,
void* priv) {
Environment* env = Environment::GetCurrent(context);
Realm* realm = Realm::GetCurrent(context);
BindingData* const binding_data =
env->AddBindingData<BindingData>(context, target);
realm->AddBindingData<BindingData>(context, target);
if (binding_data == nullptr) return;

SetMethod(context, target, "encodeInto", EncodeInto);
Expand Down
8 changes: 6 additions & 2 deletions src/encoding_binding.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ class ExternalReferenceRegistry;
namespace encoding_binding {
class BindingData : public SnapshotableObject {
public:
BindingData(Environment* env, v8::Local<v8::Object> obj);
BindingData(Realm* realm, v8::Local<v8::Object> obj);

using InternalFieldInfo = InternalFieldInfoBase;

SERIALIZABLE_OBJECT_METHODS()
SET_BINDING_ID(encoding_binding_data)

SET_NO_MEMORY_INFO()
void MemoryInfo(MemoryTracker* tracker) const override;
SET_SELF_SIZE(BindingData)
SET_MEMORY_INFO_NAME(BindingData)

Expand All @@ -35,6 +35,10 @@ class BindingData : public SnapshotableObject {
void* priv);
static void RegisterTimerExternalReferences(
ExternalReferenceRegistry* registry);

private:
static constexpr size_t kEncodeIntoResultsLength = 2;
AliasedUint32Array encode_into_results_buffer_;
};

} // namespace encoding_binding
Expand Down

0 comments on commit 6b60f38

Please sign in to comment.