Skip to content

Commit

Permalink
src: use ArrayBufferViewContents more frequently
Browse files Browse the repository at this point in the history 
Using `ArrayBufferViewContents` over
`Buffer::Data()`/`Buffer::Length()` or `SPREAD_BUFFER_ARG` has the
advantages of creating fewer individual variables to keep track off,
not being a “magic” macro that creates variables, reducing code size,
and being faster when receiving on-heap TypedArrays.

PR-URL: #27920
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
  • Loading branch information
@addaleax @targos
addaleax authored and targos committed May 31, 2019
1 parent b9cc407 commit 1a179e1
Show file tree
Hide file tree
Showing 9 changed files with 77 additions and 81 deletions.
6 changes: 3 additions & 3 deletions src/js_stream.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,9 @@ void JSStream::ReadBuffer(const FunctionCallbackInfo<Value>& args) {
JSStream* wrap;
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());

CHECK(Buffer::HasInstance(args[0]));
char* data = Buffer::Data(args[0]);
int len = Buffer::Length(args[0]);
ArrayBufferViewContents<char> buffer(args[0]);
const char* data = buffer.data();
int len = buffer.length();

// Repeatedly ask the stream's owner for memory, copy the data that we
// just read from JS into those buffers and emit them as reads.
Expand Down
80 changes: 40 additions & 40 deletions src/node_buffer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -463,17 +463,17 @@ void StringSlice(const FunctionCallbackInfo<Value>& args) {
Isolate* isolate = env->isolate();

THROW_AND_RETURN_UNLESS_BUFFER(env, args.This());
SPREAD_BUFFER_ARG(args.This(), ts_obj);
ArrayBufferViewContents<char> buffer(args.This());

if (ts_obj_length == 0)
if (buffer.length() == 0)
return args.GetReturnValue().SetEmptyString();

SLICE_START_END(env, args[0], args[1], ts_obj_length)
SLICE_START_END(env, args[0], args[1], buffer.length())

Local<Value> error;
MaybeLocal<Value> ret =
StringBytes::Encode(isolate,
ts_obj_data + start,
buffer.data() + start,
length,
encoding,
&error);
Expand All @@ -492,9 +492,8 @@ void Copy(const FunctionCallbackInfo<Value> &args) {

THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
THROW_AND_RETURN_UNLESS_BUFFER(env, args[1]);
Local<Object> buffer_obj = args[0].As<Object>();
ArrayBufferViewContents<char> source(args[0]);
Local<Object> target_obj = args[1].As<Object>();
SPREAD_BUFFER_ARG(buffer_obj, ts_obj);
SPREAD_BUFFER_ARG(target_obj, target);

size_t target_start = 0;
Expand All @@ -503,14 +502,14 @@ void Copy(const FunctionCallbackInfo<Value> &args) {

THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[2], 0, &target_start));
THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[3], 0, &source_start));
THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[4], ts_obj_length,
THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[4], source.length(),
&source_end));

// Copy 0 bytes; we're done
if (target_start >= target_length || source_start >= source_end)
return args.GetReturnValue().Set(0);

if (source_start > ts_obj_length)
if (source_start > source.length())
return THROW_ERR_OUT_OF_RANGE(
env, "The value of \"sourceStart\" is out of range.");

Expand All @@ -519,9 +518,9 @@ void Copy(const FunctionCallbackInfo<Value> &args) {

uint32_t to_copy = std::min(
std::min(source_end - source_start, target_length - target_start),
ts_obj_length - source_start);
source.length() - source_start);

memmove(target_data + target_start, ts_obj_data + source_start, to_copy);
memmove(target_data + target_start, source.data() + source_start, to_copy);
args.GetReturnValue().Set(to_copy);
}

Expand Down Expand Up @@ -689,8 +688,8 @@ void CompareOffset(const FunctionCallbackInfo<Value> &args) {

THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
THROW_AND_RETURN_UNLESS_BUFFER(env, args[1]);
SPREAD_BUFFER_ARG(args[0], ts_obj);
SPREAD_BUFFER_ARG(args[1], target);
ArrayBufferViewContents<char> source(args[0]);
ArrayBufferViewContents<char> target(args[1]);

size_t target_start = 0;
size_t source_start = 0;
Expand All @@ -699,15 +698,15 @@ void CompareOffset(const FunctionCallbackInfo<Value> &args) {

THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[2], 0, &target_start));
THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[3], 0, &source_start));
THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[4], target_length,
THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[4], target.length(),
&target_end));
THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[5], ts_obj_length,
THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[5], source.length(),
&source_end));

if (source_start > ts_obj_length)
if (source_start > source.length())
return THROW_ERR_OUT_OF_RANGE(
env, "The value of \"sourceStart\" is out of range.");
if (target_start > target_length)
if (target_start > target.length())
return THROW_ERR_OUT_OF_RANGE(
env, "The value of \"targetStart\" is out of range.");

Expand All @@ -716,11 +715,11 @@ void CompareOffset(const FunctionCallbackInfo<Value> &args) {

size_t to_cmp =
std::min(std::min(source_end - source_start, target_end - target_start),
ts_obj_length - source_start);
source.length() - source_start);

int val = normalizeCompareVal(to_cmp > 0 ?
memcmp(ts_obj_data + source_start,
target_data + target_start,
memcmp(source.data() + source_start,
target.data() + target_start,
to_cmp) : 0,
source_end - source_start,
target_end - target_start);
Expand All @@ -733,14 +732,14 @@ void Compare(const FunctionCallbackInfo<Value> &args) {

THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
THROW_AND_RETURN_UNLESS_BUFFER(env, args[1]);
SPREAD_BUFFER_ARG(args[0], obj_a);
SPREAD_BUFFER_ARG(args[1], obj_b);
ArrayBufferViewContents<char> a(args[0]);
ArrayBufferViewContents<char> b(args[1]);

size_t cmp_length = std::min(obj_a_length, obj_b_length);
size_t cmp_length = std::min(a.length(), b.length());

int val = normalizeCompareVal(cmp_length > 0 ?
memcmp(obj_a_data, obj_b_data, cmp_length) : 0,
obj_a_length, obj_b_length);
memcmp(a.data(), b.data(), cmp_length) : 0,
a.length(), b.length());
args.GetReturnValue().Set(val);
}

Expand Down Expand Up @@ -792,16 +791,16 @@ void IndexOfString(const FunctionCallbackInfo<Value>& args) {
enum encoding enc = ParseEncoding(isolate, args[3], UTF8);

THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
SPREAD_BUFFER_ARG(args[0], ts_obj);
ArrayBufferViewContents<char> buffer(args[0]);

Local<String> needle = args[1].As<String>();
int64_t offset_i64 = args[2].As<Integer>()->Value();
bool is_forward = args[4]->IsTrue();

const char* haystack = ts_obj_data;
const char* haystack = buffer.data();
// Round down to the nearest multiple of 2 in case of UCS2.
const size_t haystack_length = (enc == UCS2) ?
ts_obj_length &~ 1 : ts_obj_length; // NOLINT(whitespace/operators)
buffer.length() &~ 1 : buffer.length(); // NOLINT(whitespace/operators)

size_t needle_length;
if (!StringBytes::Size(isolate, needle, enc).To(&needle_length)) return;
Expand Down Expand Up @@ -909,15 +908,15 @@ void IndexOfBuffer(const FunctionCallbackInfo<Value>& args) {

THROW_AND_RETURN_UNLESS_BUFFER(Environment::GetCurrent(args), args[0]);
THROW_AND_RETURN_UNLESS_BUFFER(Environment::GetCurrent(args), args[1]);
SPREAD_BUFFER_ARG(args[0], ts_obj);
SPREAD_BUFFER_ARG(args[1], buf);
ArrayBufferViewContents<char> haystack_contents(args[0]);
ArrayBufferViewContents<char> needle_contents(args[1]);
int64_t offset_i64 = args[2].As<Integer>()->Value();
bool is_forward = args[4]->IsTrue();

const char* haystack = ts_obj_data;
const size_t haystack_length = ts_obj_length;
const char* needle = buf_data;
const size_t needle_length = buf_length;
const char* haystack = haystack_contents.data();
const size_t haystack_length = haystack_contents.length();
const char* needle = needle_contents.data();
const size_t needle_length = needle_contents.length();

int64_t opt_offset = IndexOfOffset(haystack_length,
offset_i64,
Expand Down Expand Up @@ -978,27 +977,28 @@ void IndexOfNumber(const FunctionCallbackInfo<Value>& args) {
CHECK(args[3]->IsBoolean());

THROW_AND_RETURN_UNLESS_BUFFER(Environment::GetCurrent(args), args[0]);
SPREAD_BUFFER_ARG(args[0], ts_obj);
ArrayBufferViewContents<char> buffer(args[0]);

uint32_t needle = args[1].As<Uint32>()->Value();
int64_t offset_i64 = args[2].As<Integer>()->Value();
bool is_forward = args[3]->IsTrue();

int64_t opt_offset = IndexOfOffset(ts_obj_length, offset_i64, 1, is_forward);
if (opt_offset <= -1 || ts_obj_length == 0) {
int64_t opt_offset =
IndexOfOffset(buffer.length(), offset_i64, 1, is_forward);
if (opt_offset <= -1 || buffer.length() == 0) {
return args.GetReturnValue().Set(-1);
}
size_t offset = static_cast<size_t>(opt_offset);
CHECK_LT(offset, ts_obj_length);
CHECK_LT(offset, buffer.length());

const void* ptr;
if (is_forward) {
ptr = memchr(ts_obj_data + offset, needle, ts_obj_length - offset);
ptr = memchr(buffer.data() + offset, needle, buffer.length() - offset);
} else {
ptr = node::stringsearch::MemrchrFill(ts_obj_data, needle, offset + 1);
ptr = node::stringsearch::MemrchrFill(buffer.data(), needle, offset + 1);
}
const char* ptr_char = static_cast<const char*>(ptr);
args.GetReturnValue().Set(ptr ? static_cast<int>(ptr_char - ts_obj_data)
args.GetReturnValue().Set(ptr ? static_cast<int>(ptr_char - buffer.data())
: -1);
}

Expand Down
27 changes: 10 additions & 17 deletions src/node_crypto.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5766,11 +5766,10 @@ void ECDH::SetPrivateKey(const FunctionCallbackInfo<Value>& args) {
ASSIGN_OR_RETURN_UNWRAP(&ecdh, args.Holder());

THROW_AND_RETURN_IF_NOT_BUFFER(env, args[0], "Private key");
ArrayBufferViewContents<unsigned char> priv_buffer(args[0]);

BignumPointer priv(BN_bin2bn(
reinterpret_cast<unsigned char*>(Buffer::Data(args[0].As<Object>())),
Buffer::Length(args[0].As<Object>()),
nullptr));
priv_buffer.data(), priv_buffer.length(), nullptr));
if (!priv)
return env->ThrowError("Failed to convert Buffer to BN");

Expand Down Expand Up @@ -6687,14 +6686,11 @@ OpenSSLBuffer ExportChallenge(const char* data, int len) {
void ExportChallenge(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

size_t len = Buffer::Length(args[0]);
if (len == 0)
ArrayBufferViewContents<char> input(args[0]);
if (input.length() == 0)
return args.GetReturnValue().SetEmptyString();

char* data = Buffer::Data(args[0]);
CHECK_NOT_NULL(data);

OpenSSLBuffer cert = ExportChallenge(data, len);
OpenSSLBuffer cert = ExportChallenge(input.data(), input.length());
if (!cert)
return args.GetReturnValue().SetEmptyString();

Expand Down Expand Up @@ -6749,16 +6745,13 @@ void ConvertKey(const FunctionCallbackInfo<Value>& args) {


void TimingSafeEqual(const FunctionCallbackInfo<Value>& args) {
CHECK(Buffer::HasInstance(args[0]));
CHECK(Buffer::HasInstance(args[1]));

size_t buf_length = Buffer::Length(args[0]);
CHECK_EQ(buf_length, Buffer::Length(args[1]));
ArrayBufferViewContents<char> buf1(args[0]);
ArrayBufferViewContents<char> buf2(args[1]);

const char* buf1 = Buffer::Data(args[0]);
const char* buf2 = Buffer::Data(args[1]);
CHECK_EQ(buf1.length(), buf2.length());

return args.GetReturnValue().Set(CRYPTO_memcmp(buf1, buf2, buf_length) == 0);
return args.GetReturnValue().Set(
CRYPTO_memcmp(buf1.data(), buf2.data(), buf1.length()) == 0);
}

void InitCryptoOnce() {
Expand Down
13 changes: 5 additions & 8 deletions src/node_http_parser_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -466,18 +466,15 @@ class Parser : public AsyncWrap, public StreamListener {
CHECK(parser->current_buffer_.IsEmpty());
CHECK_EQ(parser->current_buffer_len_, 0);
CHECK_NULL(parser->current_buffer_data_);
CHECK_EQ(Buffer::HasInstance(args[0]), true);

Local<Object> buffer_obj = args[0].As<Object>();
char* buffer_data = Buffer::Data(buffer_obj);
size_t buffer_len = Buffer::Length(buffer_obj);
ArrayBufferViewContents<char> buffer(args[0]);

// This is a hack to get the current_buffer to the callbacks with the least
// amount of overhead. Nothing else will run while http_parser_execute()
// runs, therefore this pointer can be set and used for the execution.
parser->current_buffer_ = buffer_obj;
parser->current_buffer_ = args[0].As<Object>();

Local<Value> ret = parser->Execute(buffer_data, buffer_len);
Local<Value> ret = parser->Execute(buffer.data(), buffer.length());

if (!ret.IsEmpty())
args.GetReturnValue().Set(ret);
Expand Down Expand Up @@ -643,7 +640,7 @@ class Parser : public AsyncWrap, public StreamListener {
}


Local<Value> Execute(char* data, size_t len) {
Local<Value> Execute(const char* data, size_t len) {
EscapableHandleScope scope(env()->isolate());

current_buffer_len_ = len;
Expand Down Expand Up @@ -857,7 +854,7 @@ class Parser : public AsyncWrap, public StreamListener {
bool got_exception_;
Local<Object> current_buffer_;
size_t current_buffer_len_;
char* current_buffer_data_;
const char* current_buffer_data_;
#ifdef NODE_EXPERIMENTAL_HTTP
unsigned int execute_depth_ = 0;
bool pending_pause_ = false;
Expand Down
14 changes: 6 additions & 8 deletions src/node_i18n.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,14 +205,13 @@ class ConverterObject : public BaseObject, Converter {

ConverterObject* converter;
ASSIGN_OR_RETURN_UNWRAP(&converter, args[0].As<Object>());
SPREAD_BUFFER_ARG(args[1], input_obj);
ArrayBufferViewContents<char> input(args[1]);
int flags = args[2]->Uint32Value(env->context()).ToChecked();

UErrorCode status = U_ZERO_ERROR;
MaybeStackBuffer<UChar> result;
MaybeLocal<Object> ret;
size_t limit = ucnv_getMinCharSize(converter->conv) *
input_obj_length;
size_t limit = ucnv_getMinCharSize(converter->conv) * input.length();
if (limit > 0)
result.AllocateSufficientStorage(limit);

Expand All @@ -225,8 +224,8 @@ class ConverterObject : public BaseObject, Converter {
}
});

const char* source = input_obj_data;
size_t source_length = input_obj_length;
const char* source = input.data();
size_t source_length = input.length();

if (converter->unicode_ && !converter->ignoreBOM_ && !converter->bomSeen_) {
int32_t bomOffset = 0;
Expand Down Expand Up @@ -455,8 +454,7 @@ void Transcode(const FunctionCallbackInfo<Value>&args) {
UErrorCode status = U_ZERO_ERROR;
MaybeLocal<Object> result;

CHECK(Buffer::HasInstance(args[0]));
SPREAD_BUFFER_ARG(args[0], ts_obj);
ArrayBufferViewContents<char> input(args[0]);
const enum encoding fromEncoding = ParseEncoding(isolate, args[1], BUFFER);
const enum encoding toEncoding = ParseEncoding(isolate, args[2], BUFFER);

Expand Down Expand Up @@ -490,7 +488,7 @@ void Transcode(const FunctionCallbackInfo<Value>&args) {
}

result = tfn(env, EncodingName(fromEncoding), EncodingName(toEncoding),
ts_obj_data, ts_obj_length, &status);
input.data(), input.length(), &status);
} else {
status = U_ILLEGAL_ARGUMENT_ERROR;
}
Expand Down
4 changes: 2 additions & 2 deletions src/node_serdes.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,8 @@ void SerializerContext::WriteRawBytes(const FunctionCallbackInfo<Value>& args) {
ctx->env(), "source must be a TypedArray or a DataView");
}

ctx->serializer_.WriteRawBytes(Buffer::Data(args[0]),
Buffer::Length(args[0]));
ArrayBufferViewContents<char> bytes(args[0]);
ctx->serializer_.WriteRawBytes(bytes.data(), bytes.length());
}

DeserializerContext::DeserializerContext(Environment* env,
Expand Down
6 changes: 3 additions & 3 deletions src/tls_wrap.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,9 @@ void TLSWrap::Receive(const FunctionCallbackInfo<Value>& args) {
TLSWrap* wrap;
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());

CHECK(Buffer::HasInstance(args[0]));
char* data = Buffer::Data(args[0]);
size_t len = Buffer::Length(args[0]);
ArrayBufferViewContents<char> buffer(args[0]);
const char* data = buffer.data();
size_t len = buffer.length();
Debug(wrap, "Receiving %zu bytes injected from JS", len);

// Copy given buffer entirely or partiall if handle becomes closed
Expand Down
7 changes: 7 additions & 0 deletions src/util-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,13 @@ ArrayBufferViewContents<T, S>::ArrayBufferViewContents(
Read(value.As<v8::ArrayBufferView>());
}

template <typename T, size_t S>
ArrayBufferViewContents<T, S>::ArrayBufferViewContents(
v8::Local<v8::Object> value) {
CHECK(value->IsArrayBufferView());
Read(value.As<v8::ArrayBufferView>());
}

template <typename T, size_t S>
ArrayBufferViewContents<T, S>::ArrayBufferViewContents(
v8::Local<v8::ArrayBufferView> abv) {
Expand Down
Loading

0 comments on commit 1a179e1

Please sign in to comment.