Skip to content
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.

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

http2, async-wrap: introduce AliasedBuffer class #15077

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@
'src/util.cc',
'src/uv.cc',
# headers to make for a more pleasant IDE experience
'src/aliased_buffer.h',
'src/async-wrap.h',
'src/async-wrap-inl.h',
'src/base-object.h',
Expand All @@ -246,6 +247,7 @@
'src/node_constants.h',
'src/node_debug_options.h',
'src/node_http2.h',
'src/node_http2_state.h',
'src/node_internals.h',
'src/node_javascript.h',
'src/node_mutex.h',
Expand Down Expand Up @@ -650,6 +652,8 @@
'sources': [
'src/node_platform.cc',
'src/node_platform.h',
'test/cctest/node_test_fixture.cc',
'test/cctest/test_aliased_buffer.cc',
'test/cctest/test_base64.cc',
'test/cctest/test_environment.cc',
'test/cctest/test_util.cc',
Expand Down
200 changes: 200 additions & 0 deletions src/aliased_buffer.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@

#ifndef SRC_ALIASED_BUFFER_H_
#define SRC_ALIASED_BUFFER_H_

#include "v8.h"
#include "util.h"
#include "util-inl.h"

namespace node {

/**
* This class encapsulates the technique of having a native buffer mapped to
* a JS object. Writes to the native buffer can happen efficiently without
* going through JS, and the data is then available to user's via the exposed
* JS object.
*
* While this technique is computationaly efficient, it is effectively a
* write to JS program state w/out going through the standard
* (monitored) API. Thus any VM capabilities to detect the modification are
* circumvented.
*
* The encapsulation herein provides a placeholder where such writes can be
* observed. Any notification APIs will be left as a future exercise.
*/
template <class NativeT, class V8T>
class AliasedBuffer {
public:
AliasedBuffer(v8::Isolate* isolate, const size_t count)
: isolate_(isolate),
count_(count),
byte_offset_(0),
free_buffer_(true) {
CHECK_GT(count, 0);
const v8::HandleScope handle_scope(isolate_);

const size_t sizeInBytes = sizeof(NativeT) * count;

// allocate native buffer
buffer_ = Calloc<NativeT>(count);
Copy link
Contributor

Choose a reason for hiding this comment

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

if we don't expect a value of count == 0 then please CHECK_GT(count, 0). if we do expect that possibility then please directly assign nullptr because UncheckedCalloc() will still force an count = 1 allocation.

Copy link
Author

Choose a reason for hiding this comment

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

added CHECK_GT(count, 0)


// allocate v8 ArrayBuffer
v8::Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(
isolate_, buffer_, sizeInBytes);

// allocate v8 TypedArray
v8::Local<V8T> js_array = V8T::New(ab, byte_offset_, count);
js_array_ = v8::Global<V8T>(isolate, js_array);
}

/**
* Create an AliasedBuffer over a sub-region of another aliased buffer.
* The two will share a v8::ArrayBuffer instance &
* a native buffer, but will each read/write to different sections of the
* native buffer.
*
* Note that byte_offset must by aligned by sizeof(NativeT).
*/
AliasedBuffer(v8::Isolate* isolate,
const size_t byte_offset,
const size_t count,
const AliasedBuffer<uint8_t,
v8::Uint8Array>& backing_buffer)
: isolate_(isolate),
count_(count),
byte_offset_(byte_offset),
free_buffer_(false) {
const v8::HandleScope handle_scope(isolate_);

v8::Local<v8::ArrayBuffer> ab = backing_buffer.GetArrayBuffer();

// validate that the byte_offset is aligned with sizeof(NativeT)
CHECK_EQ(byte_offset & (sizeof(NativeT) - 1), 0);
// validate this fits inside the backing buffer
CHECK_LE(sizeof(NativeT) * count, ab->ByteLength() - byte_offset);

buffer_ = reinterpret_cast<NativeT*>(
const_cast<uint8_t*>(backing_buffer.GetNativeBuffer() + byte_offset));

v8::Local<V8T> js_array = V8T::New(ab, byte_offset, count);
js_array_ = v8::Global<V8T>(isolate, js_array);
}

AliasedBuffer(const AliasedBuffer& that)
: isolate_(that.isolate_),
count_(that.count_),
byte_offset_(that.byte_offset_),
buffer_(that.buffer_),
free_buffer_(false) {
js_array_ = v8::Global<V8T>(that.isolate_, that.GetJSArray());
}

~AliasedBuffer() {
if (free_buffer_ && buffer_ != nullptr) {
free(buffer_);
}
js_array_.Reset();
}

/**
* Helper class that is returned from operator[] to support assignment into
* a specified location.
*/
class Reference {
public:
Reference(AliasedBuffer<NativeT, V8T>* aliased_buffer, size_t index)
: aliased_buffer_(aliased_buffer),
index_(index) {
}

Reference(const Reference& that)
: aliased_buffer_(that.aliased_buffer_),
index_(that.index_) {
}

inline Reference& operator=(const NativeT &val) {
aliased_buffer_->SetValue(index_, val);
return *this;
}

operator NativeT() const {
return aliased_buffer_->GetValue(index_);
}

private:
AliasedBuffer<NativeT, V8T>* aliased_buffer_;
size_t index_;
};

/**
* Get the underlying v8 TypedArray overlayed on top of the native buffer
*/
v8::Local<V8T> GetJSArray() const {
return js_array_.Get(isolate_);
}

/**
* Get the underlying v8::ArrayBuffer underlying the TypedArray and
* overlaying the native buffer
*/
v8::Local<v8::ArrayBuffer> GetArrayBuffer() const {
return GetJSArray()->Buffer();
}

/**
* Get the underlying native buffer. Note that all reads/writes should occur
* through the GetValue/SetValue/operator[] methods
*/
inline const NativeT* GetNativeBuffer() const {
return buffer_;
}

/**
* Synonym for GetBuffer()
*/
inline const NativeT* operator * () const {
return GetNativeBuffer();
}

/**
* Set position index to given value.
*/
inline void SetValue(const size_t index, NativeT value) {
#if defined(DEBUG) && DEBUG
CHECK_LT(index, count_);
#endif
buffer_[index] = value;
}

/**
* Get value at position index
*/
inline const NativeT GetValue(const size_t index) const {
#if defined(DEBUG) && DEBUG
CHECK_LT(index, count_);
#endif
return buffer_[index];
}

/**
* Effectively, a synonym for GetValue/SetValue
*/
Reference operator[](size_t index) {
return Reference(this, index);
}

NativeT operator[](size_t index) const {
return GetValue(index);
}

private:
v8::Isolate* const isolate_;
size_t count_;
size_t byte_offset_;
NativeT* buffer_;
v8::Global<V8T> js_array_;
bool free_buffer_;
};
} // namespace node

#endif // SRC_ALIASED_BUFFER_H_
16 changes: 2 additions & 14 deletions src/async-wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
#include "v8-profiler.h"

using v8::Array;
using v8::ArrayBuffer;
using v8::Context;
using v8::Float64Array;
using v8::Function;
Expand All @@ -53,7 +52,6 @@ using v8::RetainedObjectInfo;
using v8::String;
using v8::Symbol;
using v8::TryCatch;
using v8::Uint32Array;
using v8::Undefined;
using v8::Value;

Expand Down Expand Up @@ -512,13 +510,9 @@ void AsyncWrap::Initialize(Local<Object> target,
// callbacks waiting to be called on a particular event. It can then be
// incremented/decremented from JS quickly to communicate to C++ if there are
// any callbacks waiting to be called.
uint32_t* fields_ptr = env->async_hooks()->fields();
int fields_count = env->async_hooks()->fields_count();
Local<ArrayBuffer> fields_ab =
ArrayBuffer::New(isolate, fields_ptr, fields_count * sizeof(*fields_ptr));
FORCE_SET_TARGET_FIELD(target,
"async_hook_fields",
Uint32Array::New(fields_ab, 0, fields_count));
env->async_hooks()->fields().GetJSArray());

// The following v8::Float64Array has 5 fields. These fields are shared in
// this way to allow JS and C++ to read/write each value as quickly as
Expand All @@ -529,15 +523,9 @@ void AsyncWrap::Initialize(Local<Object> target,
// kInitTriggerId: Write the id of the resource responsible for a handle's
// creation just before calling the new handle's constructor. After the new
// handle is constructed kInitTriggerId is set back to 0.
double* uid_fields_ptr = env->async_hooks()->uid_fields();
int uid_fields_count = env->async_hooks()->uid_fields_count();
Local<ArrayBuffer> uid_fields_ab = ArrayBuffer::New(
isolate,
uid_fields_ptr,
uid_fields_count * sizeof(*uid_fields_ptr));
FORCE_SET_TARGET_FIELD(target,
"async_uid_fields",
Float64Array::New(uid_fields_ab, 0, uid_fields_count));
env->async_hooks()->uid_fields().GetJSArray());

Local<Object> constants = Object::New(isolate);
#define SET_HOOKS_CONSTANT(name) \
Expand Down
32 changes: 19 additions & 13 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

#include "aliased_buffer.h"
#include "env.h"
#include "node.h"
#include "util.h"
Expand Down Expand Up @@ -82,8 +83,8 @@ inline uint32_t* IsolateData::zero_fill_field() const {

inline Environment::AsyncHooks::AsyncHooks(v8::Isolate* isolate)
: isolate_(isolate),
fields_(),
uid_fields_() {
fields_(isolate, kFieldsCount),
uid_fields_(isolate, kUidFieldsCount) {
v8::HandleScope handle_scope(isolate_);

// kAsyncUidCntr should start at 1 because that'll be the id the execution
Expand All @@ -105,15 +106,17 @@ inline Environment::AsyncHooks::AsyncHooks(v8::Isolate* isolate)
#undef V
}

inline uint32_t* Environment::AsyncHooks::fields() {
inline AliasedBuffer<uint32_t, v8::Uint32Array>&
Environment::AsyncHooks::fields() {
return fields_;
}

inline int Environment::AsyncHooks::fields_count() const {
return kFieldsCount;
}

inline double* Environment::AsyncHooks::uid_fields() {
inline AliasedBuffer<double, v8::Float64Array>&
Environment::AsyncHooks::uid_fields() {
return uid_fields_;
}

Expand Down Expand Up @@ -147,7 +150,7 @@ inline bool Environment::AsyncHooks::pop_ids(double async_id) {
fprintf(stderr,
"Error: async hook stack has become corrupted ("
"actual: %.f, expected: %.f)\n",
uid_fields_[kCurrentAsyncId],
uid_fields_.GetValue(kCurrentAsyncId),
async_id);
Environment* env = Environment::GetCurrent(isolate_);
DumpBacktrace(stderr);
Expand Down Expand Up @@ -346,7 +349,7 @@ inline Environment::~Environment() {
delete[] heap_statistics_buffer_;
delete[] heap_space_statistics_buffer_;
delete[] http_parser_buffer_;
free(http2_state_buffer_);
delete http2_state_;
free(performance_state_);
}

Expand Down Expand Up @@ -445,7 +448,9 @@ inline std::vector<double>* Environment::destroy_ids_list() {
}

inline double Environment::new_async_id() {
return ++async_hooks()->uid_fields()[AsyncHooks::kAsyncUidCntr];
async_hooks()->uid_fields()[AsyncHooks::kAsyncUidCntr] =
async_hooks()->uid_fields()[AsyncHooks::kAsyncUidCntr] + 1;
return async_hooks()->uid_fields()[AsyncHooks::kAsyncUidCntr];
Copy link
Contributor

Choose a reason for hiding this comment

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

is this only a style change, or does the previous operation not work anymore?

Copy link
Member

@addaleax addaleax Sep 5, 2017

Choose a reason for hiding this comment

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

The previous operation doesn’t work anymore, we’d need to overload operator++ for that, and from there it quickly starts to devolve into supporting all integer operations

}

inline double Environment::current_async_id() {
Expand All @@ -457,7 +462,8 @@ inline double Environment::trigger_id() {
}

inline double Environment::get_init_trigger_id() {
double* uid_fields = async_hooks()->uid_fields();
AliasedBuffer<double, v8::Float64Array>& uid_fields =
async_hooks()->uid_fields();
double tid = uid_fields[AsyncHooks::kInitTriggerId];
uid_fields[AsyncHooks::kInitTriggerId] = 0;
if (tid <= 0) tid = current_async_id();
Expand Down Expand Up @@ -497,13 +503,13 @@ inline void Environment::set_http_parser_buffer(char* buffer) {
http_parser_buffer_ = buffer;
}

inline http2::http2_state* Environment::http2_state_buffer() const {
return http2_state_buffer_;
inline http2::http2_state* Environment::http2_state() const {
return http2_state_;
}

inline void Environment::set_http2_state_buffer(http2::http2_state* buffer) {
CHECK_EQ(http2_state_buffer_, nullptr); // Should be set only once.
http2_state_buffer_ = buffer;
inline void Environment::set_http2_state(http2::http2_state* buffer) {
CHECK_EQ(http2_state_, nullptr); // Should be set only once.
http2_state_ = buffer;
}

inline double* Environment::fs_stats_field_array() const {
Expand Down
Loading