Skip to content

Commit

Permalink
src: turn AllocatedBuffer into thin wrapper around v8::BackingStore
Browse files Browse the repository at this point in the history
Alternative to #33381 that
reimplements that change on top of moving AllocatedBuffer out
of env.h

PR-URL: #33291
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
  • Loading branch information
jasnell committed May 30, 2020
1 parent 56ff1ee commit e2cd715
Show file tree
Hide file tree
Showing 9 changed files with 116 additions and 194 deletions.
128 changes: 64 additions & 64 deletions src/allocated_buffer-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,97 +12,97 @@

namespace node {

// It's a bit awkward to define this Buffer::New() overload here, but it
// avoids a circular dependency with node_internals.h.
namespace Buffer {
v8::MaybeLocal<v8::Uint8Array> New(Environment* env,
v8::Local<v8::ArrayBuffer> ab,
size_t byte_offset,
size_t length);
}

NoArrayBufferZeroFillScope::NoArrayBufferZeroFillScope(
IsolateData* isolate_data)
: node_allocator_(isolate_data->node_allocator()) {
if (node_allocator_ != nullptr) node_allocator_->zero_fill_field()[0] = 0;
}

NoArrayBufferZeroFillScope::~NoArrayBufferZeroFillScope() {
if (node_allocator_ != nullptr) node_allocator_->zero_fill_field()[0] = 1;
}

AllocatedBuffer AllocatedBuffer::AllocateManaged(
Environment* env,
size_t size,
int flags) {
char* data = flags & ALLOCATE_MANAGED_UNCHECKED ?
env->AllocateUnchecked(size) :
env->Allocate(size);
if (data == nullptr) size = 0;
return AllocatedBuffer(env, uv_buf_init(data, size));
size_t size) {
NoArrayBufferZeroFillScope no_zero_fill_scope(env->isolate_data());
std::unique_ptr<v8::BackingStore> bs =
v8::ArrayBuffer::NewBackingStore(env->isolate(), size);
return AllocatedBuffer(env, std::move(bs));
}

inline AllocatedBuffer::AllocatedBuffer(Environment* env, uv_buf_t buf)
: env_(env), buffer_(buf) {}
AllocatedBuffer::AllocatedBuffer(
Environment* env, std::unique_ptr<v8::BackingStore> bs)
: env_(env), backing_store_(std::move(bs)) {}

AllocatedBuffer::AllocatedBuffer(
Environment* env, uv_buf_t buffer)
: env_(env) {
if (buffer.base == nullptr) return;
auto map = env->released_allocated_buffers();
auto it = map->find(buffer.base);
CHECK_NE(it, map->end());
backing_store_ = std::move(it->second);
map->erase(it);
}

inline void AllocatedBuffer::Resize(size_t len) {
// The `len` check is to make sure we don't end up with `nullptr` as our base.
char* new_data = env_->Reallocate(buffer_.base, buffer_.len,
len > 0 ? len : 1);
CHECK_NOT_NULL(new_data);
buffer_ = uv_buf_init(new_data, len);
void AllocatedBuffer::Resize(size_t len) {
if (len == 0) {
backing_store_ = v8::ArrayBuffer::NewBackingStore(env_->isolate(), 0);
return;
}
NoArrayBufferZeroFillScope no_zero_fill_scope(env_->isolate_data());
backing_store_ = v8::BackingStore::Reallocate(
env_->isolate(), std::move(backing_store_), len);
}

inline uv_buf_t AllocatedBuffer::release() {
uv_buf_t ret = buffer_;
buffer_ = uv_buf_init(nullptr, 0);
if (data() == nullptr) return uv_buf_init(nullptr, 0);

CHECK_NOT_NULL(env_);
uv_buf_t ret = uv_buf_init(data(), size());
env_->released_allocated_buffers()->emplace(
ret.base, std::move(backing_store_));
return ret;
}

inline char* AllocatedBuffer::data() {
return buffer_.base;
if (!backing_store_) return nullptr;
return static_cast<char*>(backing_store_->Data());
}

inline const char* AllocatedBuffer::data() const {
return buffer_.base;
if (!backing_store_) return nullptr;
return static_cast<char*>(backing_store_->Data());
}

inline size_t AllocatedBuffer::size() const {
return buffer_.len;
}

inline AllocatedBuffer::AllocatedBuffer(Environment* env)
: env_(env), buffer_(uv_buf_init(nullptr, 0)) {}

inline AllocatedBuffer::AllocatedBuffer(AllocatedBuffer&& other)
: AllocatedBuffer() {
*this = std::move(other);
}

inline AllocatedBuffer& AllocatedBuffer::operator=(AllocatedBuffer&& other) {
clear();
env_ = other.env_;
buffer_ = other.release();
return *this;
}

inline AllocatedBuffer::~AllocatedBuffer() {
clear();
inline size_t AllocatedBuffer::size() const {
if (!backing_store_) return 0;
return backing_store_->ByteLength();
}

inline void AllocatedBuffer::clear() {
uv_buf_t buf = release();
if (buf.base != nullptr) {
CHECK_NOT_NULL(env_);
env_->Free(buf.base, buf.len);
}
backing_store_.reset();
}

inline v8::MaybeLocal<v8::Object> AllocatedBuffer::ToBuffer() {
CHECK_NOT_NULL(env_);
v8::MaybeLocal<v8::Object> obj = Buffer::New(env_, data(), size(), false);
if (!obj.IsEmpty()) release();
return obj;
v8::Local<v8::ArrayBuffer> ab = ToArrayBuffer();
return Buffer::New(env_, ab, 0, ab->ByteLength())
.FromMaybe(v8::Local<v8::Uint8Array>());
}

inline v8::Local<v8::ArrayBuffer> AllocatedBuffer::ToArrayBuffer() {
CHECK_NOT_NULL(env_);
uv_buf_t buf = release();
auto callback = [](void* data, size_t length, void* deleter_data){
CHECK_NOT_NULL(deleter_data);

static_cast<v8::ArrayBuffer::Allocator*>(deleter_data)
->Free(data, length);
};
std::unique_ptr<v8::BackingStore> backing =
v8::ArrayBuffer::NewBackingStore(buf.base,
buf.len,
callback,
env_->isolate()
->GetArrayBufferAllocator());
return v8::ArrayBuffer::New(env_->isolate(),
std::move(backing));
return v8::ArrayBuffer::New(env_->isolate(), std::move(backing_store_));
}

} // namespace node
Expand Down
48 changes: 29 additions & 19 deletions src/allocated_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,31 +6,43 @@
#include "base_object.h"
#include "uv.h"
#include "v8.h"
#include "env.h"

namespace node {

class Environment;

// Disables zero-filling for ArrayBuffer allocations in this scope. This is
// similar to how we implement Buffer.allocUnsafe() in JS land.
class NoArrayBufferZeroFillScope{
public:
inline explicit NoArrayBufferZeroFillScope(IsolateData* isolate_data);
inline ~NoArrayBufferZeroFillScope();

private:
NodeArrayBufferAllocator* node_allocator_;

friend class Environment;
};

// A unique-pointer-ish object that is compatible with the JS engine's
// ArrayBuffer::Allocator.
// TODO(addaleax): We may want to start phasing this out as it's only a
// thin wrapper around v8::BackingStore at this point
struct AllocatedBuffer {
public:
enum AllocateManagedFlags {
ALLOCATE_MANAGED_FLAG_NONE,
ALLOCATE_MANAGED_UNCHECKED
};

// Utilities that allocate memory using the Isolate's ArrayBuffer::Allocator.
// In particular, using AllocateManaged() will provide a RAII-style object
// with easy conversion to `Buffer` and `ArrayBuffer` objects.
inline static AllocatedBuffer AllocateManaged(
Environment* env,
size_t size,
int flags = ALLOCATE_MANAGED_FLAG_NONE);

explicit inline AllocatedBuffer(Environment* env = nullptr);
inline AllocatedBuffer(Environment* env, uv_buf_t buf);
inline ~AllocatedBuffer();
inline static AllocatedBuffer AllocateManaged(Environment* env, size_t size);

AllocatedBuffer() = default;
inline AllocatedBuffer(
Environment* env, std::unique_ptr<v8::BackingStore> bs);
// For this constructor variant, `buffer` *must* come from an earlier call
// to .release
inline AllocatedBuffer(Environment* env, uv_buf_t buffer);

inline void Resize(size_t len);

inline uv_buf_t release();
Expand All @@ -42,16 +54,14 @@ struct AllocatedBuffer {
inline v8::MaybeLocal<v8::Object> ToBuffer();
inline v8::Local<v8::ArrayBuffer> ToArrayBuffer();

inline AllocatedBuffer(AllocatedBuffer&& other);
inline AllocatedBuffer& operator=(AllocatedBuffer&& other);
AllocatedBuffer(AllocatedBuffer&& other) = default;
AllocatedBuffer& operator=(AllocatedBuffer&& other) = default;
AllocatedBuffer(const AllocatedBuffer& other) = delete;
AllocatedBuffer& operator=(const AllocatedBuffer& other) = delete;

private:
Environment* env_;
// We do not pass this to libuv directly, but uv_buf_t is a convenient way
// to represent a chunk of memory, and plays nicely with other parts of core.
uv_buf_t buffer_;
Environment* env_ = nullptr;
std::unique_ptr<v8::BackingStore> backing_store_;

friend class Environment;
};
Expand Down
26 changes: 3 additions & 23 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -867,29 +867,9 @@ inline IsolateData* Environment::isolate_data() const {
return isolate_data_;
}

inline char* Environment::AllocateUnchecked(size_t size) {
return static_cast<char*>(
isolate_data()->allocator()->AllocateUninitialized(size));
}

inline char* Environment::Allocate(size_t size) {
char* ret = AllocateUnchecked(size);
CHECK_NE(ret, nullptr);
return ret;
}

inline void Environment::Free(char* data, size_t size) {
if (data != nullptr)
isolate_data()->allocator()->Free(data, size);
}

// It's a bit awkward to define this Buffer::New() overload here, but it
// avoids a circular dependency with node_internals.h.
namespace Buffer {
v8::MaybeLocal<v8::Object> New(Environment* env,
char* data,
size_t length,
bool uses_malloc);
std::unordered_map<char*, std::unique_ptr<v8::BackingStore>>*
Environment::released_allocated_buffers() {
return &released_allocated_buffers_;
}

inline void Environment::ThrowError(const char* errmsg) {
Expand Down
19 changes: 0 additions & 19 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1104,25 +1104,6 @@ void Environment::MemoryInfo(MemoryTracker* tracker) const {
// node, we shift its sizeof() size out of the Environment node.
}

char* Environment::Reallocate(char* data, size_t old_size, size_t size) {
if (old_size == size) return data;
// If we know that the allocator is our ArrayBufferAllocator, we can let
// if reallocate directly.
if (isolate_data()->uses_node_allocator()) {
return static_cast<char*>(
isolate_data()->node_allocator()->Reallocate(data, old_size, size));
}
// Generic allocators do not provide a reallocation method; we need to
// allocate a new chunk of memory and copy the data over.
char* new_data = AllocateUnchecked(size);
if (new_data == nullptr) return nullptr;
memcpy(new_data, data, std::min(size, old_size));
if (size > old_size)
memset(new_data + old_size, 0, size - old_size);
Free(data, old_size);
return new_data;
}

void Environment::RunWeakRefCleanup() {
isolate()->ClearKeptObjects();
}
Expand Down
13 changes: 8 additions & 5 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -926,11 +926,6 @@ class Environment : public MemoryRetainer {

inline IsolateData* isolate_data() const;

inline char* Allocate(size_t size);
inline char* AllocateUnchecked(size_t size);
char* Reallocate(char* data, size_t old_size, size_t size);
inline void Free(char* data, size_t size);

inline bool printed_error() const;
inline void set_printed_error(bool value);

Expand Down Expand Up @@ -1218,6 +1213,9 @@ class Environment : public MemoryRetainer {
void RunAndClearNativeImmediates(bool only_refed = false);
void RunAndClearInterrupts();

inline std::unordered_map<char*, std::unique_ptr<v8::BackingStore>>*
released_allocated_buffers();

private:
inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
const char* errmsg);
Expand Down Expand Up @@ -1378,6 +1376,11 @@ class Environment : public MemoryRetainer {
// We should probably find a way to just use plain `v8::String`s created from
// the source passed to LoadEnvironment() directly instead.
std::unique_ptr<v8::String::Value> main_utf16_;

// Used by AllocatedBuffer::release() to keep track of the BackingStore for
// a given pointer.
std::unordered_map<char*, std::unique_ptr<v8::BackingStore>>
released_allocated_buffers_;
};

} // namespace node
Expand Down
Loading

0 comments on commit e2cd715

Please sign in to comment.