Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
src: add allocation utils to env
Add a RAII utility for managing blocks of memory that have
been allocated with the `ArrayBuffer::Allocator` for a given
`Isolate`.
  • Loading branch information
addaleax committed Feb 21, 2019
commit 41e81b1d063ba8fbe446701a96c839cfb122d0e1
99 changes: 99 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,105 @@ 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);
}

inline AllocatedBuffer Environment::AllocateManaged(size_t size, bool checked) {
char* data = checked ? Allocate(size) : AllocateUnchecked(size);
if (data == nullptr) size = 0;
return AllocatedBuffer(this, uv_buf_init(data, size));
}

inline AllocatedBuffer::AllocatedBuffer(Environment* env, uv_buf_t buf)
: env_(env), buffer_(buf) {}

inline void AllocatedBuffer::Resize(size_t len) {
char* new_data = env_->Reallocate(buffer_.base, buffer_.len, len);
CHECK_IMPLIES(len > 0, new_data != nullptr);
buffer_.base = new_data;
buffer_.len = len;
}

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

inline char* AllocatedBuffer::data() {
return buffer_.base;
}

inline const char* AllocatedBuffer::data() const {
return buffer_.base;
}

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 void AllocatedBuffer::clear() {
uv_buf_t buf = release();
env_->Free(buf.base, buf.len);
}

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

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

inline v8::Local<v8::ArrayBuffer> AllocatedBuffer::ToArrayBuffer() {
CHECK_NOT_NULL(env_);
uv_buf_t buf = release();
return v8::ArrayBuffer::New(env_->isolate(),
buf.base,
buf.len,
v8::ArrayBufferCreationMode::kInternalized);
}

inline void Environment::ThrowError(const char* errmsg) {
ThrowError(v8::Exception::Error, errmsg);
}
Expand Down
18 changes: 18 additions & 0 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ namespace node {

using errors::TryCatchScope;
using v8::Boolean;
using v8::ArrayBuffer;
using v8::Context;
using v8::EmbedderGraph;
using v8::External;
Expand Down Expand Up @@ -905,6 +906,23 @@ void Environment::BuildEmbedderGraph(Isolate* isolate,
});
}

char* Environment::Reallocate(char* data, size_t old_size, size_t size) {
// 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;
}

// Not really any better place than env.cc at this moment.
void BaseObject::DeleteMe(void* data) {
Expand Down
39 changes: 39 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,36 @@ enum class DebugCategory {
CATEGORY_COUNT
};

// A unique-pointer-ish object that is compatible with the JS engine's
// ArrayBuffer::Allocator.
struct AllocatedBuffer {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

AllocatedLibuvBuffer? Or instead of storing a uv_buf_t as member, store the fields that can be used to wrap/unwrap uv_buf_t?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

AllocatedLibuvBuffer?

So, I’m not super happy with the name either. I wouldn’t want to include a reference to libuv, because uv_but_t was just chosen here as a nicer variant of std::pair<char*,size_t> that happens to work nicely with some pieces in our code base that already use it the same way.

Ideally, the name would be something that refers to the fact that this memory is usable for ArrayBuffers, but that would make any name even longer … ArrayBufferBuffer sounds weird, ArrayBufferStorage or ArrayBufferMemory is long but maybe okay, …?

Or instead of storing a uv_buf_t as member, store the fields that can be used to wrap/unwrap uv_buf_t?

I don’t really have strong feelings about that… I can make that change after/while rebasing (later today, after #26201 lands).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

because uv_but_t was just chosen here as a nicer variant of std::pair<char*,size_t> that happens to work nicely with some pieces in our code base that already use it the same way.

That was what I guessed as well, thanks for the explanation. I don't have strong feelings about using uv_buf_t underneath, but it would be nice to mention why it is chosen in the comments as one may think that AllocatedBuffer depends on libuv for a more sophisticated reason.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@joyeecheung Does 1e17828 sound good to you?

(& to be clear, I’m still happy about suggestions for another name for this class if that fits better.)

public:
explicit inline AllocatedBuffer(Environment* env = nullptr);
inline AllocatedBuffer(Environment* env, uv_buf_t buf);
inline ~AllocatedBuffer();
inline void Resize(size_t len);

inline uv_buf_t release();
inline char* data();
inline const char* data() const;
inline size_t size() const;
inline void clear();

inline v8::MaybeLocal<v8::Object> ToBuffer();
inline v8::Local<v8::ArrayBuffer> ToArrayBuffer();

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

private:
Environment* env_;
uv_buf_t buffer_;

friend class Environment;
};

class Environment {
public:
class AsyncHooks {
Expand Down Expand Up @@ -697,6 +727,15 @@ class Environment {

inline IsolateData* isolate_data() const;

// Utilites 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 AllocatedBuffer AllocateManaged(size_t size, bool checked = true);
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