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

src: use predefined AliasedBuffer types in the code base #27334

Closed
wants to merge 1 commit into from
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
58 changes: 31 additions & 27 deletions src/aliased_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@

#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

#include "v8.h"
#include <cinttypes>
#include "util-inl.h"
#include "v8.h"

namespace node {

/**
* Do not use this class directly when creating instances of it - use the
* Aliased*Array defined at the end of this file instead.
*
* 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
Expand All @@ -22,18 +26,16 @@ namespace node {
* 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,
template <class NativeT,
class V8T,
// SFINAE NativeT to be scalar
typename = std::enable_if_t<std::is_scalar<NativeT>::value>>
class AliasedBuffer {
class AliasedBufferBase {
public:
AliasedBuffer(v8::Isolate* isolate, const size_t count)
: isolate_(isolate),
count_(count),
byte_offset_(0) {
AliasedBufferBase(v8::Isolate* isolate, const size_t count)
: isolate_(isolate), count_(count), byte_offset_(0) {
CHECK_GT(count, 0);
const v8::HandleScope handle_scope(isolate_);

const size_t size_in_bytes =
MultiplyWithOverflowCheck(sizeof(NativeT), count);

Expand All @@ -48,22 +50,20 @@ class AliasedBuffer {
}

/**
* Create an AliasedBuffer over a sub-region of another aliased buffer.
* Create an AliasedBufferBase 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).
*/
// TODO(refack): refactor into a non-owning `AliasedBufferView`
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) {
// TODO(refack): refactor into a non-owning `AliasedBufferBaseView`
AliasedBufferBase(
v8::Isolate* isolate,
const size_t byte_offset,
const size_t count,
const AliasedBufferBase<uint8_t, v8::Uint8Array>& backing_buffer)
: isolate_(isolate), count_(count), byte_offset_(byte_offset) {
const v8::HandleScope handle_scope(isolate_);

v8::Local<v8::ArrayBuffer> ab = backing_buffer.GetArrayBuffer();
Expand All @@ -81,16 +81,16 @@ class AliasedBuffer {
js_array_ = v8::Global<V8T>(isolate, js_array);
}

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

AliasedBuffer& operator=(AliasedBuffer&& that) noexcept {
this->~AliasedBuffer();
AliasedBufferBase& operator=(AliasedBufferBase&& that) noexcept {
this->~AliasedBufferBase();
isolate_ = that.isolate_;
count_ = that.count_;
byte_offset_ = that.byte_offset_;
Expand All @@ -109,10 +109,8 @@ class AliasedBuffer {
*/
class Reference {
public:
Reference(AliasedBuffer<NativeT, V8T>* aliased_buffer, size_t index)
: aliased_buffer_(aliased_buffer),
index_(index) {
}
Reference(AliasedBufferBase<NativeT, V8T>* aliased_buffer, size_t index)
: aliased_buffer_(aliased_buffer), index_(index) {}

Reference(const Reference& that)
: aliased_buffer_(that.aliased_buffer_),
Expand Down Expand Up @@ -149,7 +147,7 @@ class AliasedBuffer {
}

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

Expand Down Expand Up @@ -216,7 +214,7 @@ class AliasedBuffer {

// Should only be used to extend the array.
// Should only be used on an owning array, not one created as a sub array of
// an owning `AliasedBuffer`.
// an owning `AliasedBufferBase`.
void reserve(size_t new_capacity) {
DCHECK_GE(new_capacity, count_);
DCHECK_EQ(byte_offset_, 0);
Expand Down Expand Up @@ -251,6 +249,12 @@ class AliasedBuffer {
NativeT* buffer_;
v8::Global<V8T> js_array_;
};

typedef AliasedBufferBase<int32_t, v8::Int32Array> AliasedInt32Array;
Copy link
Member Author

@joyeecheung joyeecheung Apr 21, 2019

Choose a reason for hiding this comment

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

Another reason for doing this is that it would be easier to implement something like AliasedInt32Array::ForEach() if we add a static list in the case like what we do for the BaseObjects - then we'll be able to iterate over all known aliased buffers in the heap snapshot generator, as well as do bookkeeping for snapshot [de]serialization (we'll record the indexes of the TypedArrays and ArrayBuffers in the snapshot during serialization, and deserialize them into C++ later to keep the states consistent)

typedef AliasedBufferBase<uint8_t, v8::Uint8Array> AliasedUint8Array;
typedef AliasedBufferBase<uint32_t, v8::Uint32Array> AliasedUint32Array;
typedef AliasedBufferBase<double, v8::Float64Array> AliasedFloat64Array;
typedef AliasedBufferBase<uint64_t, v8::BigUint64Array> AliasedBigUint64Array;
} // namespace node

#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
Expand Down
24 changes: 9 additions & 15 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,16 +101,15 @@ inline AsyncHooks::AsyncHooks()
NODE_ASYNC_PROVIDER_TYPES(V)
#undef V
}

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

inline AliasedBuffer<double, v8::Float64Array>& AsyncHooks::async_id_fields() {
inline AliasedFloat64Array& AsyncHooks::async_id_fields() {
return async_id_fields_;
}

inline AliasedBuffer<double, v8::Float64Array>& AsyncHooks::async_ids_stack() {
inline AliasedFloat64Array& AsyncHooks::async_ids_stack() {
return async_ids_stack_;
}

Expand Down Expand Up @@ -235,8 +234,7 @@ inline void Environment::PopAsyncCallbackScope() {
inline ImmediateInfo::ImmediateInfo(v8::Isolate* isolate)
: fields_(isolate, kFieldsCount) {}

inline AliasedBuffer<uint32_t, v8::Uint32Array>&
ImmediateInfo::fields() {
inline AliasedUint32Array& ImmediateInfo::fields() {
return fields_;
}

Expand Down Expand Up @@ -271,7 +269,7 @@ inline void ImmediateInfo::ref_count_dec(uint32_t decrement) {
inline TickInfo::TickInfo(v8::Isolate* isolate)
: fields_(isolate, kFieldsCount) {}

inline AliasedBuffer<uint8_t, v8::Uint8Array>& TickInfo::fields() {
inline AliasedUint8Array& TickInfo::fields() {
return fields_;
}

Expand Down Expand Up @@ -478,13 +476,11 @@ inline void Environment::set_abort_on_uncaught_exception(bool value) {
options_->abort_on_uncaught_exception = value;
}

inline AliasedBuffer<uint32_t, v8::Uint32Array>&
Environment::should_abort_on_uncaught_toggle() {
inline AliasedUint32Array& Environment::should_abort_on_uncaught_toggle() {
return should_abort_on_uncaught_toggle_;
}

inline AliasedBuffer<int32_t, v8::Int32Array>&
Environment::stream_base_state() {
inline AliasedInt32Array& Environment::stream_base_state() {
return stream_base_state_;
}

Expand Down Expand Up @@ -614,13 +610,11 @@ void Environment::set_debug_enabled(DebugCategory category, bool enabled) {
debug_enabled_[static_cast<int>(category)] = enabled;
}

inline AliasedBuffer<double, v8::Float64Array>*
Environment::fs_stats_field_array() {
inline AliasedFloat64Array* Environment::fs_stats_field_array() {
return &fs_stats_field_array_;
}

inline AliasedBuffer<uint64_t, v8::BigUint64Array>*
Environment::fs_stats_field_bigint_array() {
inline AliasedBigUint64Array* Environment::fs_stats_field_bigint_array() {
return &fs_stats_field_bigint_array_;
}

Expand Down
38 changes: 18 additions & 20 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -605,9 +605,9 @@ class AsyncHooks : public MemoryRetainer {
kUidFieldsCount,
};

inline AliasedBuffer<uint32_t, v8::Uint32Array>& fields();
inline AliasedBuffer<double, v8::Float64Array>& async_id_fields();
inline AliasedBuffer<double, v8::Float64Array>& async_ids_stack();
inline AliasedUint32Array& fields();
inline AliasedFloat64Array& async_id_fields();
inline AliasedFloat64Array& async_ids_stack();

inline v8::Local<v8::String> provider_string(int idx);

Expand Down Expand Up @@ -646,12 +646,12 @@ class AsyncHooks : public MemoryRetainer {
// Keep a list of all Persistent strings used for Provider types.
std::array<v8::Eternal<v8::String>, AsyncWrap::PROVIDERS_LENGTH> providers_;
// Stores the ids of the current execution context stack.
AliasedBuffer<double, v8::Float64Array> async_ids_stack_;
AliasedFloat64Array async_ids_stack_;
// Attached to a Uint32Array that tracks the number of active hooks for
// each type.
AliasedBuffer<uint32_t, v8::Uint32Array> fields_;
AliasedUint32Array fields_;
// Attached to a Float64Array that tracks the state of async resources.
AliasedBuffer<double, v8::Float64Array> async_id_fields_;
AliasedFloat64Array async_id_fields_;

void grow_async_ids_stack();
};
Expand All @@ -670,7 +670,7 @@ class AsyncCallbackScope {

class ImmediateInfo : public MemoryRetainer {
public:
inline AliasedBuffer<uint32_t, v8::Uint32Array>& fields();
inline AliasedUint32Array& fields();
inline uint32_t count() const;
inline uint32_t ref_count() const;
inline bool has_outstanding() const;
Expand All @@ -692,12 +692,12 @@ class ImmediateInfo : public MemoryRetainer {

enum Fields { kCount, kRefCount, kHasOutstanding, kFieldsCount };

AliasedBuffer<uint32_t, v8::Uint32Array> fields_;
AliasedUint32Array fields_;
};

class TickInfo : public MemoryRetainer {
public:
inline AliasedBuffer<uint8_t, v8::Uint8Array>& fields();
inline AliasedUint8Array& fields();
inline bool has_tick_scheduled() const;
inline bool has_rejection_to_warn() const;

Expand All @@ -714,7 +714,7 @@ class TickInfo : public MemoryRetainer {

enum Fields { kHasTickScheduled = 0, kHasRejectionToWarn, kFieldsCount };

AliasedBuffer<uint8_t, v8::Uint8Array> fields_;
AliasedUint8Array fields_;
};

class TrackingTraceStateObserver :
Expand Down Expand Up @@ -902,10 +902,9 @@ class Environment : public MemoryRetainer {
// This is a pseudo-boolean that keeps track of whether an uncaught exception
// should abort the process or not if --abort-on-uncaught-exception was
// passed to Node. If the flag was not passed, it is ignored.
inline AliasedBuffer<uint32_t, v8::Uint32Array>&
should_abort_on_uncaught_toggle();
inline AliasedUint32Array& should_abort_on_uncaught_toggle();

inline AliasedBuffer<int32_t, v8::Int32Array>& stream_base_state();
inline AliasedInt32Array& stream_base_state();

// The necessary API for async_hooks.
inline double new_async_id();
Expand Down Expand Up @@ -951,9 +950,8 @@ class Environment : public MemoryRetainer {
inline void set_debug_enabled(DebugCategory category, bool enabled);
void set_debug_categories(const std::string& cats, bool enabled);

inline AliasedBuffer<double, v8::Float64Array>* fs_stats_field_array();
inline AliasedBuffer<uint64_t, v8::BigUint64Array>*
fs_stats_field_bigint_array();
inline AliasedFloat64Array* fs_stats_field_array();
inline AliasedBigUint64Array* fs_stats_field_bigint_array();

inline std::vector<std::unique_ptr<fs::FileHandleReadWrap>>&
file_handle_read_wrap_freelist();
Expand Down Expand Up @@ -1192,12 +1190,12 @@ class Environment : public MemoryRetainer {
uint32_t script_id_counter_ = 0;
uint32_t function_id_counter_ = 0;

AliasedBuffer<uint32_t, v8::Uint32Array> should_abort_on_uncaught_toggle_;
AliasedUint32Array should_abort_on_uncaught_toggle_;
int should_not_abort_scope_counter_ = 0;

std::unique_ptr<TrackingTraceStateObserver> trace_state_observer_;

AliasedBuffer<int32_t, v8::Int32Array> stream_base_state_;
AliasedInt32Array stream_base_state_;

std::unique_ptr<performance::performance_state> performance_state_;
std::unordered_map<std::string, uint64_t> performance_marks_;
Expand Down Expand Up @@ -1240,8 +1238,8 @@ class Environment : public MemoryRetainer {

bool debug_enabled_[static_cast<int>(DebugCategory::CATEGORY_COUNT)] = {0};

AliasedBuffer<double, v8::Float64Array> fs_stats_field_array_;
AliasedBuffer<uint64_t, v8::BigUint64Array> fs_stats_field_bigint_array_;
AliasedFloat64Array fs_stats_field_array_;
AliasedBigUint64Array fs_stats_field_bigint_array_;

std::vector<std::unique_ptr<fs::FileHandleReadWrap>>
file_handle_read_wrap_freelist_;
Expand Down
2 changes: 1 addition & 1 deletion src/memory_tracker-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ void MemoryTracker::TrackField(const char* name,

template <class NativeT, class V8T>
void MemoryTracker::TrackField(const char* name,
const AliasedBuffer<NativeT, V8T>& value,
const AliasedBufferBase<NativeT, V8T>& value,
const char* node_name) {
TrackField(name, value.GetJSArray(), "AliasedBuffer");
}
Expand Down
2 changes: 1 addition & 1 deletion src/memory_tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ class MemoryTracker {
const char* node_name = nullptr);
template <class NativeT, class V8T>
inline void TrackField(const char* edge_name,
const AliasedBuffer<NativeT, V8T>& value,
const AliasedBufferBase<NativeT, V8T>& value,
const char* node_name = nullptr);

// Put a memory container into the graph, create an edge from
Expand Down
4 changes: 2 additions & 2 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -756,9 +756,9 @@ inline FSReqBase* GetReqWrap(Environment* env, Local<Value> value,
return Unwrap<FSReqBase>(value.As<Object>());
} else if (value->StrictEquals(env->fs_use_promises_symbol())) {
if (use_bigint) {
return FSReqPromise<uint64_t, BigUint64Array>::New(env, use_bigint);
return FSReqPromise<AliasedBigUint64Array>::New(env, use_bigint);
} else {
return FSReqPromise<double, Float64Array>::New(env, use_bigint);
return FSReqPromise<AliasedFloat64Array>::New(env, use_bigint);
}
}
return nullptr;
Expand Down
9 changes: 5 additions & 4 deletions src/node_file.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,9 @@ constexpr uint64_t ToNative(uv_timespec_t ts) {
#undef constexpr // end N3652 bug workaround

template <typename NativeT, typename V8T>
constexpr void FillStatsArray(AliasedBuffer<NativeT, V8T>* fields,
const uv_stat_t* s, const size_t offset = 0) {
constexpr void FillStatsArray(AliasedBufferBase<NativeT, V8T>* fields,
const uv_stat_t* s,
const size_t offset = 0) {
fields->SetValue(offset + 0, static_cast<NativeT>(s->st_dev));
fields->SetValue(offset + 1, static_cast<NativeT>(s->st_mode));
fields->SetValue(offset + 2, static_cast<NativeT>(s->st_nlink));
Expand Down Expand Up @@ -227,7 +228,7 @@ inline Local<Value> FillGlobalStatsArray(Environment* env,
}
}

template <typename NativeT = double, typename V8T = v8::Float64Array>
template <typename AliasedBufferT>
class FSReqPromise : public FSReqBase {
public:
static FSReqPromise* New(Environment* env, bool use_bigint) {
Expand Down Expand Up @@ -304,7 +305,7 @@ class FSReqPromise : public FSReqBase {
stats_field_array_(env->isolate(), kFsStatsFieldsNumber) {}

bool finished_ = false;
AliasedBuffer<NativeT, V8T> stats_field_array_;
AliasedBufferT stats_field_array_;
};

class FSReqAfterScope {
Expand Down
Loading