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.

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

Jump to bottom

src: separate owning and non-owning AliasedBuffers #33142

Closed
wants to merge 1 commit into from
Closed

src: separate owning and non-owning AliasedBuffers #33142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
215 changes: 118 additions & 97 deletions src/aliased_buffer.h
View file
Open in desktop
Expand Up @@ -9,6 +9,18 @@

namespace node {

template <class NativeT,
class V8T,
// SFINAE NativeT to be scalar
typename Trait = std::enable_if_t<std::is_scalar<NativeT>::value>>
class OwningAliasedBufferBase;

template <class NativeT,
class V8T,
// SFINAE NativeT to be scalar
typename Trait = std::enable_if_t<std::is_scalar<NativeT>::value>>
class AliasedBufferViewBase;
Copy link
Member

Choose a reason for hiding this comment

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

Either some code comments explaining the differences and why one would be used vs. the other or some information added to src/README.md explaining the same (or both) would be ideal


/**
* Do not use this class directly when creating instances of it - use the
* Aliased*Array defined at the end of this file instead.
Expand All @@ -26,83 +38,9 @@ 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,
// SFINAE NativeT to be scalar
typename = std::enable_if_t<std::is_scalar<NativeT>::value>>
template <class NativeT, class V8T>
class AliasedBufferBase {
public:
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);

// allocate v8 ArrayBuffer
v8::Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(
isolate_, size_in_bytes);
buffer_ = static_cast<NativeT*>(ab->GetBackingStore()->Data());

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

/**
* 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 `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();

// 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(MultiplyWithOverflowCheck(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);
}

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());
}

AliasedBufferBase& operator=(AliasedBufferBase&& that) noexcept {
this->~AliasedBufferBase();
isolate_ = that.isolate_;
count_ = that.count_;
byte_offset_ = that.byte_offset_;
buffer_ = that.buffer_;

js_array_.Reset(isolate_, that.js_array_.Get(isolate_));

that.buffer_ = nullptr;
that.js_array_.Reset();
return *this;
}

/**
* Helper class that is returned from operator[] to support assignment into
* a specified location.
Expand Down Expand Up @@ -212,50 +150,133 @@ class AliasedBufferBase {
return count_;
}

friend class OwningAliasedBufferBase<NativeT, V8T>;
friend class AliasedBufferViewBase<NativeT, V8T>;

private:
AliasedBufferBase(v8::Isolate* isolate, const size_t count)
: isolate_(isolate), count_(count) {}

v8::Isolate* isolate_;
size_t count_;
NativeT* buffer_;
v8::Global<V8T> js_array_;
};

template <class NativeT, class V8T, typename Trait>
class OwningAliasedBufferBase : public AliasedBufferBase<NativeT, V8T> {
typedef AliasedBufferBase<NativeT, V8T> BaseType;

public:
OwningAliasedBufferBase(v8::Isolate* isolate, const size_t count)
: BaseType(isolate, count) {
CHECK_GT(count, 0);
const v8::HandleScope handle_scope(isolate);
const size_t size_in_bytes =
MultiplyWithOverflowCheck(sizeof(NativeT), count);

// allocate v8 ArrayBuffer
v8::Local<v8::ArrayBuffer> ab =
v8::ArrayBuffer::New(isolate, size_in_bytes);
BaseType::buffer_ = static_cast<NativeT*>(ab->GetBackingStore()->Data());

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

// 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 `AliasedBufferBase`.
void reserve(size_t new_capacity) {
DCHECK_GE(new_capacity, count_);
DCHECK_EQ(byte_offset_, 0);
const v8::HandleScope handle_scope(isolate_);
size_t count = BaseType::count_;
v8::Isolate* isolate = BaseType::isolate_;
DCHECK_GE(new_capacity, count);
const v8::HandleScope handle_scope(isolate);

const size_t old_size_in_bytes = sizeof(NativeT) * count_;
const size_t old_size_in_bytes = sizeof(NativeT) * count;
const size_t new_size_in_bytes = MultiplyWithOverflowCheck(sizeof(NativeT),
new_capacity);

// allocate v8 new ArrayBuffer
v8::Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(
isolate_, new_size_in_bytes);
v8::Local<v8::ArrayBuffer> ab =
v8::ArrayBuffer::New(isolate, new_size_in_bytes);

// allocate new native buffer
NativeT* new_buffer = static_cast<NativeT*>(ab->GetBackingStore()->Data());
// copy old content
memcpy(new_buffer, buffer_, old_size_in_bytes);
memcpy(new_buffer, BaseType::buffer_, old_size_in_bytes);

// allocate v8 TypedArray
v8::Local<V8T> js_array = V8T::New(ab, byte_offset_, new_capacity);
v8::Local<V8T> js_array = V8T::New(ab, 0, new_capacity);

// move over old v8 TypedArray
js_array_ = std::move(v8::Global<V8T>(isolate_, js_array));
BaseType::js_array_ = std::move(v8::Global<V8T>(isolate, js_array));

BaseType::buffer_ = new_buffer;
BaseType::count_ = new_capacity;
}
OwningAliasedBufferBase(const OwningAliasedBufferBase&) = delete;
OwningAliasedBufferBase& operator=(const OwningAliasedBufferBase&) = delete;
OwningAliasedBufferBase(OwningAliasedBufferBase&&) = delete;
OwningAliasedBufferBase& operator=(OwningAliasedBufferBase&&) = delete;
};

/**
* Create an AliasedBufferViewBase over a sub-region of another
* AliasedBufferBase. 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).
*/
template <class NativeT, class V8T, typename Trait>
class AliasedBufferViewBase : public AliasedBufferBase<NativeT, V8T> {
typedef AliasedBufferBase<NativeT, V8T> BaseType;

public:
AliasedBufferViewBase(
v8::Isolate* isolate,
const size_t byte_offset,
const size_t count,
const AliasedBufferBase<uint8_t, v8::Uint8Array>& backing_buffer)
: BaseType(isolate, count), byte_offset_(byte_offset) {
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(MultiplyWithOverflowCheck(sizeof(NativeT), count),
ab->ByteLength() - byte_offset);

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

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

AliasedBufferViewBase(const AliasedBufferViewBase&) = delete;
AliasedBufferViewBase& operator=(const AliasedBufferViewBase&) = delete;
AliasedBufferViewBase(AliasedBufferViewBase&&) = delete;
AliasedBufferViewBase& operator=(AliasedBufferViewBase&&) = delete;

private:
v8::Isolate* isolate_;
size_t count_;
size_t byte_offset_;
NativeT* buffer_;
v8::Global<V8T> js_array_;
};

typedef AliasedBufferBase<int32_t, v8::Int32Array> AliasedInt32Array;
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;
#define ALIASED_BUFFER_TYPES(V) \
V(uint8_t, Uint8Array) \
V(int32_t, Int32Array) \
V(uint32_t, Uint32Array) \
V(double, Float64Array) \
V(uint64_t, BigUint64Array)

#define V(NativeT, V8T) \
typedef OwningAliasedBufferBase<NativeT, v8::V8T> OwningAliased##V8T; \
typedef AliasedBufferViewBase<NativeT, v8::V8T> Aliased##V8T##View;
ALIASED_BUFFER_TYPES(V)
#undef V
} // namespace node

#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
Expand Down
15 changes: 8 additions & 7 deletions src/env-inl.h
View file
Open in desktop
Expand Up @@ -100,15 +100,15 @@ inline AsyncHooks::AsyncHooks()
// context during bootstrap (code that runs before entering uv_run()).
async_id_fields_[AsyncHooks::kAsyncIdCounter] = 1;
}
inline AliasedUint32Array& AsyncHooks::fields() {
inline OwningAliasedUint32Array& AsyncHooks::fields() {
return fields_;
}

inline AliasedFloat64Array& AsyncHooks::async_id_fields() {
inline OwningAliasedFloat64Array& AsyncHooks::async_id_fields() {
return async_id_fields_;
}

inline AliasedFloat64Array& AsyncHooks::async_ids_stack() {
inline OwningAliasedFloat64Array& AsyncHooks::async_ids_stack() {
return async_ids_stack_;
}

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

inline AliasedUint32Array& ImmediateInfo::fields() {
inline OwningAliasedUint32Array& ImmediateInfo::fields() {
return fields_;
}

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

inline AliasedUint8Array& TickInfo::fields() {
inline OwningAliasedUint8Array& TickInfo::fields() {
return fields_;
}

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

inline AliasedUint32Array& Environment::should_abort_on_uncaught_toggle() {
inline OwningAliasedUint32Array&
Environment::should_abort_on_uncaught_toggle() {
return should_abort_on_uncaught_toggle_;
}

inline AliasedInt32Array& Environment::stream_base_state() {
inline OwningAliasedInt32Array& Environment::stream_base_state() {
return stream_base_state_;
}

Expand Down