Skip to content

encoding: use AliasedUint32Array for encodeInto results #46658

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
joyeecheung wants to merge 5 commits into from
Closed

encoding: use AliasedUint32Array for encodeInto results #46658

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
29ef3b0
benchmark: stablize encode benchmark
joyeecheung Feb 11, 2023
57bb217
src: move encoding bindings to a new binding
joyeecheung Feb 11, 2023
9b54d80
encoding: use AliasedUint32Array for encodeInto results
joyeecheung Feb 11, 2023
1233e7f
fixup! encoding: use AliasedUint32Array for encodeInto results
joyeecheung Mar 7, 2023
4ea7f2e
fixup! src: move encoding bindings to a new binding
joyeecheung Mar 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
encoding: use AliasedUint32Array for encodeInto results 
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.
  • Loading branch information
@joyeecheung
joyeecheung committed Feb 28, 2023
commit 9b54d805cea39244fdfb9c1d6a3542d2f076782a
15 changes: 8 additions & 7 deletions lib/internal/encoding.js
View file
Open in desktop
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);
Comment thread
anonrig marked this conversation as resolved.

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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,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 @@ -48,19 +63,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) {

@debadree25 debadree25 May 5, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity I exploring places where fast API could be applied, came across this function and PR, I experimented a bit it seems v8::FastApiTypedArray<uint8_t>& is indeed supported and it does get caught in fast call, but I am not sure how one could extract ->Buffer() from v8::FastApiTypedArray<uint8_t>&, is that even possible? just wondering do let me know your thoughts cc @anonrig @joyeecheung

Thank You!

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 @@ -69,21 +84,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 @@ -176,9 +186,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
View file
Open in desktop
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