Skip to content

Commit

Permalink
src: allow not materializing ArrayBuffers from C++
Browse files Browse the repository at this point in the history
Where appropriate, use a helper that wraps around
`ArrayBufferView::Buffer()` or `ArrayBufferView::CopyContents()`
rather than `Buffer::Data()`, as that may help to avoid materializing
the underlying `ArrayBuffer` when reading small typed arrays from C++.
This allows keeping the performance benefits of the faster creation of
heap-allocated small typed arrays in many cases.

PR-URL: #26301
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
addaleax committed Mar 1, 2019
1 parent e2baa68 commit 9b4eec0
Show file tree
Hide file tree
Showing 8 changed files with 183 additions and 145 deletions.
227 changes: 107 additions & 120 deletions src/node_crypto.cc

Large diffs are not rendered by default.

7 changes: 3 additions & 4 deletions src/node_crypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ class SSLWrap {

ClientHelloParser hello_parser_;

Persistent<v8::Object> ocsp_response_;
Persistent<v8::ArrayBufferView> ocsp_response_;
Persistent<v8::Value> sni_context_;

friend class SecureContext;
Expand Down Expand Up @@ -462,7 +462,7 @@ class KeyObject : public BaseObject {
static void New(const v8::FunctionCallbackInfo<v8::Value>& args);

static void Init(const v8::FunctionCallbackInfo<v8::Value>& args);
void InitSecret(const char* key, size_t key_len);
void InitSecret(v8::Local<v8::ArrayBufferView> abv);
void InitPublic(const ManagedEVPPKey& pkey);
void InitPrivate(const ManagedEVPPKey& pkey);

Expand Down Expand Up @@ -803,8 +803,7 @@ class ECDH : public BaseObject {
static void Initialize(Environment* env, v8::Local<v8::Object> target);
static ECPointPointer BufferToPoint(Environment* env,
const EC_GROUP* group,
char* data,
size_t len);
v8::Local<v8::Value> buf);

// TODO(joyeecheung): track the memory used by OpenSSL types
SET_NO_MEMORY_INFO()
Expand Down
26 changes: 12 additions & 14 deletions src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace node {

using v8::ArrayBuffer;
using v8::ArrayBufferView;
using v8::Boolean;
using v8::Context;
using v8::Float64Array;
Expand Down Expand Up @@ -2483,7 +2484,7 @@ void Http2Session::Request(const FunctionCallbackInfo<Value>& args) {
// state of the Http2Session, it's simply a notification.
void Http2Session::Goaway(uint32_t code,
int32_t lastStreamID,
uint8_t* data,
const uint8_t* data,
size_t len) {
if (IsDestroyed())
return;
Expand All @@ -2508,16 +2509,13 @@ void Http2Session::Goaway(const FunctionCallbackInfo<Value>& args) {

uint32_t code = args[0]->Uint32Value(context).ToChecked();
int32_t lastStreamID = args[1]->Int32Value(context).ToChecked();
Local<Value> opaqueData = args[2];
uint8_t* data = nullptr;
size_t length = 0;
ArrayBufferViewContents<uint8_t> opaque_data;

if (Buffer::HasInstance(opaqueData)) {
data = reinterpret_cast<uint8_t*>(Buffer::Data(opaqueData));
length = Buffer::Length(opaqueData);
if (args[2]->IsArrayBufferView()) {
opaque_data.Read(args[2].As<ArrayBufferView>());
}

session->Goaway(code, lastStreamID, data, length);
session->Goaway(code, lastStreamID, opaque_data.data(), opaque_data.length());
}

// Update accounting of data chunks. This is used primarily to manage timeout
Expand Down Expand Up @@ -2772,10 +2770,10 @@ void Http2Session::Ping(const FunctionCallbackInfo<Value>& args) {

// A PING frame may have exactly 8 bytes of payload data. If not provided,
// then the current hrtime will be used as the payload.
uint8_t* payload = nullptr;
if (Buffer::HasInstance(args[0])) {
payload = reinterpret_cast<uint8_t*>(Buffer::Data(args[0]));
CHECK_EQ(Buffer::Length(args[0]), 8);
ArrayBufferViewContents<uint8_t, 8> payload;
if (args[0]->IsArrayBufferView()) {
payload.Read(args[0].As<ArrayBufferView>());
CHECK_EQ(payload.length(), 8);
}

Local<Object> obj;
Expand All @@ -2800,7 +2798,7 @@ void Http2Session::Ping(const FunctionCallbackInfo<Value>& args) {
// the callback will be invoked and a notification sent out to JS land. The
// notification will include the duration of the ping, allowing the round
// trip to be measured.
ping->Send(payload);
ping->Send(payload.data());
args.GetReturnValue().Set(true);
}

Expand Down Expand Up @@ -2872,7 +2870,7 @@ Http2Session::Http2Ping::Http2Ping(Http2Session* session, Local<Object> obj)
session_(session),
startTime_(uv_hrtime()) {}

void Http2Session::Http2Ping::Send(uint8_t* payload) {
void Http2Session::Http2Ping::Send(const uint8_t* payload) {
uint8_t data[8];
if (payload == nullptr) {
memcpy(&data, &startTime_, arraysize(data));
Expand Down
5 changes: 3 additions & 2 deletions src/node_http2.h
Original file line number Diff line number Diff line change
Expand Up @@ -699,7 +699,8 @@ class Http2Session : public AsyncWrap, public StreamListener {
void Close(uint32_t code = NGHTTP2_NO_ERROR,
bool socket_closed = false);
void Consume(Local<External> external);
void Goaway(uint32_t code, int32_t lastStreamID, uint8_t* data, size_t len);
void Goaway(uint32_t code, int32_t lastStreamID,
const uint8_t* data, size_t len);
void AltSvc(int32_t id,
uint8_t* origin,
size_t origin_len,
Expand Down Expand Up @@ -1089,7 +1090,7 @@ class Http2Session::Http2Ping : public AsyncWrap {
SET_MEMORY_INFO_NAME(Http2Ping)
SET_SELF_SIZE(Http2Ping)

void Send(uint8_t* payload);
void Send(const uint8_t* payload);
void Done(bool ack, const uint8_t* payload = nullptr);

private:
Expand Down
9 changes: 7 additions & 2 deletions src/string_decoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "string_decoder-inl.h"

using v8::Array;
using v8::ArrayBufferView;
using v8::Context;
using v8::FunctionCallbackInfo;
using v8::Integer;
Expand Down Expand Up @@ -252,9 +253,13 @@ void DecodeData(const FunctionCallbackInfo<Value>& args) {
StringDecoder* decoder =
reinterpret_cast<StringDecoder*>(Buffer::Data(args[0]));
CHECK_NOT_NULL(decoder);
size_t nread = Buffer::Length(args[1]);

CHECK(args[1]->IsArrayBufferView());
ArrayBufferViewContents<char> content(args[1].As<ArrayBufferView>());
size_t length = content.length();

MaybeLocal<String> ret =
decoder->DecodeData(args.GetIsolate(), Buffer::Data(args[1]), &nread);
decoder->DecodeData(args.GetIsolate(), content.data(), &length);
if (!ret.IsEmpty())
args.GetReturnValue().Set(ret.ToLocalChecked());
}
Expand Down
26 changes: 26 additions & 0 deletions src/util-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,32 @@ SlicedArguments::SlicedArguments(
(*this)[i] = args[i + start];
}

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

template <typename T, size_t S>
ArrayBufferViewContents<T, S>::ArrayBufferViewContents(
v8::Local<v8::ArrayBufferView> abv) {
Read(abv);
}

template <typename T, size_t S>
void ArrayBufferViewContents<T, S>::Read(v8::Local<v8::ArrayBufferView> abv) {
static_assert(sizeof(T) == 1, "Only supports one-byte data at the moment");
length_ = abv->ByteLength();
if (length_ > sizeof(stack_storage_) || abv->HasBuffer()) {
data_ = static_cast<T*>(abv->Buffer()->GetContents().Data()) +
abv->ByteOffset();
} else {
abv->CopyContents(stack_storage_, sizeof(stack_storage_));
data_ = stack_storage_;
}
}

} // namespace node

#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
Expand Down
7 changes: 4 additions & 3 deletions src/util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

namespace node {

using v8::ArrayBufferView;
using v8::Isolate;
using v8::Local;
using v8::String;
Expand Down Expand Up @@ -89,11 +90,11 @@ BufferValue::BufferValue(Isolate* isolate, Local<Value> value) {

if (value->IsString()) {
MakeUtf8String(isolate, value, this);
} else if (Buffer::HasInstance(value)) {
const size_t len = Buffer::Length(value);
} else if (value->IsArrayBufferView()) {
const size_t len = value.As<ArrayBufferView>()->ByteLength();
// Leave place for the terminating '\0' byte.
AllocateSufficientStorage(len + 1);
memcpy(out(), Buffer::Data(value), len);
value.As<ArrayBufferView>()->CopyContents(out(), len);
SetLengthAndZeroTerminate(len);
} else {
Invalidate();
Expand Down
21 changes: 21 additions & 0 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,27 @@ class MaybeStackBuffer {
T buf_st_[kStackStorageSize];
};

// Provides access to an ArrayBufferView's storage, either the original,
// or for small data, a copy of it. This object's lifetime is bound to the
// original ArrayBufferView's lifetime.
template <typename T, size_t kStackStorageSize = 64>
class ArrayBufferViewContents {
public:
ArrayBufferViewContents() {}

explicit inline ArrayBufferViewContents(v8::Local<v8::Value> value);
explicit inline ArrayBufferViewContents(v8::Local<v8::ArrayBufferView> abv);
inline void Read(v8::Local<v8::ArrayBufferView> abv);

inline const T* data() const { return data_; }
inline size_t length() const { return length_; }

private:
T stack_storage_[kStackStorageSize];
T* data_ = nullptr;
size_t length_ = 0;
};

class Utf8Value : public MaybeStackBuffer<char> {
public:
explicit Utf8Value(v8::Isolate* isolate, v8::Local<v8::Value> value);
Expand Down

0 comments on commit 9b4eec0

Please sign in to comment.