Skip to content

Commit e2cd715

Browse files
jasnelljasnell
committed
src: turn AllocatedBuffer into thin wrapper around v8::BackingStore
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>
1 parent 56ff1ee commit e2cd715

9 files changed

Lines changed: 116 additions & 194 deletions

File tree

‎src/allocated_buffer-inl.h‎

Lines changed: 64 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -12,97 +12,97 @@
1212

1313
namespace node {
1414

15+
// It's a bit awkward to define this Buffer::New() overload here, but it
16+
// avoids a circular dependency with node_internals.h.
17+
namespace Buffer {
18+
v8::MaybeLocal<v8::Uint8Array> New(Environment* env,
19+
v8::Local<v8::ArrayBuffer> ab,
20+
size_t byte_offset,
21+
size_t length);
22+
}
23+
24+
NoArrayBufferZeroFillScope::NoArrayBufferZeroFillScope(
25+
IsolateData* isolate_data)
26+
: node_allocator_(isolate_data->node_allocator()) {
27+
if (node_allocator_ != nullptr) node_allocator_->zero_fill_field()[0] = 0;
28+
}
29+
30+
NoArrayBufferZeroFillScope::~NoArrayBufferZeroFillScope() {
31+
if (node_allocator_ != nullptr) node_allocator_->zero_fill_field()[0] = 1;
32+
}
33+
1534
AllocatedBuffer AllocatedBuffer::AllocateManaged(
1635
Environment* env,
17-
size_t size,
18-
int flags) {
19-
char* data = flags & ALLOCATE_MANAGED_UNCHECKED ?
20-
env->AllocateUnchecked(size) :
21-
env->Allocate(size);
22-
if (data == nullptr) size = 0;
23-
return AllocatedBuffer(env, uv_buf_init(data, size));
36+
size_t size) {
37+
NoArrayBufferZeroFillScope no_zero_fill_scope(env->isolate_data());
38+
std::unique_ptr<v8::BackingStore> bs =
39+
v8::ArrayBuffer::NewBackingStore(env->isolate(), size);
40+
return AllocatedBuffer(env, std::move(bs));
2441
}
2542

26-
inline AllocatedBuffer::AllocatedBuffer(Environment* env, uv_buf_t buf)
27-
: env_(env), buffer_(buf) {}
43+
AllocatedBuffer::AllocatedBuffer(
44+
Environment* env, std::unique_ptr<v8::BackingStore> bs)
45+
: env_(env), backing_store_(std::move(bs)) {}
46+
47+
AllocatedBuffer::AllocatedBuffer(
48+
Environment* env, uv_buf_t buffer)
49+
: env_(env) {
50+
if (buffer.base == nullptr) return;
51+
auto map = env->released_allocated_buffers();
52+
auto it = map->find(buffer.base);
53+
CHECK_NE(it, map->end());
54+
backing_store_ = std::move(it->second);
55+
map->erase(it);
56+
}
2857

29-
inline void AllocatedBuffer::Resize(size_t len) {
30-
// The `len` check is to make sure we don't end up with `nullptr` as our base.
31-
char* new_data = env_->Reallocate(buffer_.base, buffer_.len,
32-
len > 0 ? len : 1);
33-
CHECK_NOT_NULL(new_data);
34-
buffer_ = uv_buf_init(new_data, len);
58+
void AllocatedBuffer::Resize(size_t len) {
59+
if (len == 0) {
60+
backing_store_ = v8::ArrayBuffer::NewBackingStore(env_->isolate(), 0);
61+
return;
62+
}
63+
NoArrayBufferZeroFillScope no_zero_fill_scope(env_->isolate_data());
64+
backing_store_ = v8::BackingStore::Reallocate(
65+
env_->isolate(), std::move(backing_store_), len);
3566
}
3667

3768
inline uv_buf_t AllocatedBuffer::release() {
38-
uv_buf_t ret = buffer_;
39-
buffer_ = uv_buf_init(nullptr, 0);
69+
if (data() == nullptr) return uv_buf_init(nullptr, 0);
70+
71+
CHECK_NOT_NULL(env_);
72+
uv_buf_t ret = uv_buf_init(data(), size());
73+
env_->released_allocated_buffers()->emplace(
74+
ret.base, std::move(backing_store_));
4075
return ret;
4176
}
4277

4378
inline char* AllocatedBuffer::data() {
44-
return buffer_.base;
79+
if (!backing_store_) return nullptr;
80+
return static_cast<char*>(backing_store_->Data());
4581
}
4682

4783
inline const char* AllocatedBuffer::data() const {
48-
return buffer_.base;
84+
if (!backing_store_) return nullptr;
85+
return static_cast<char*>(backing_store_->Data());
4986
}
5087

51-
inline size_t AllocatedBuffer::size() const {
52-
return buffer_.len;
53-
}
54-
55-
inline AllocatedBuffer::AllocatedBuffer(Environment* env)
56-
: env_(env), buffer_(uv_buf_init(nullptr, 0)) {}
57-
58-
inline AllocatedBuffer::AllocatedBuffer(AllocatedBuffer&& other)
59-
: AllocatedBuffer() {
60-
*this = std::move(other);
61-
}
62-
63-
inline AllocatedBuffer& AllocatedBuffer::operator=(AllocatedBuffer&& other) {
64-
clear();
65-
env_ = other.env_;
66-
buffer_ = other.release();
67-
return *this;
68-
}
6988

70-
inline AllocatedBuffer::~AllocatedBuffer() {
71-
clear();
89+
inline size_t AllocatedBuffer::size() const {
90+
if (!backing_store_) return 0;
91+
return backing_store_->ByteLength();
7292
}
7393

7494
inline void AllocatedBuffer::clear() {
75-
uv_buf_t buf = release();
76-
if (buf.base != nullptr) {
77-
CHECK_NOT_NULL(env_);
78-
env_->Free(buf.base, buf.len);
79-
}
95+
backing_store_.reset();
8096
}
8197

8298
inline v8::MaybeLocal<v8::Object> AllocatedBuffer::ToBuffer() {
83-
CHECK_NOT_NULL(env_);
84-
v8::MaybeLocal<v8::Object> obj = Buffer::New(env_, data(), size(), false);
85-
if (!obj.IsEmpty()) release();
86-
return obj;
99+
v8::Local<v8::ArrayBuffer> ab = ToArrayBuffer();
100+
return Buffer::New(env_, ab, 0, ab->ByteLength())
101+
.FromMaybe(v8::Local<v8::Uint8Array>());
87102
}
88103

89104
inline v8::Local<v8::ArrayBuffer> AllocatedBuffer::ToArrayBuffer() {
90-
CHECK_NOT_NULL(env_);
91-
uv_buf_t buf = release();
92-
auto callback = [](void* data, size_t length, void* deleter_data){
93-
CHECK_NOT_NULL(deleter_data);
94-
95-
static_cast<v8::ArrayBuffer::Allocator*>(deleter_data)
96-
->Free(data, length);
97-
};
98-
std::unique_ptr<v8::BackingStore> backing =
99-
v8::ArrayBuffer::NewBackingStore(buf.base,
100-
buf.len,
101-
callback,
102-
env_->isolate()
103-
->GetArrayBufferAllocator());
104-
return v8::ArrayBuffer::New(env_->isolate(),
105-
std::move(backing));
105+
return v8::ArrayBuffer::New(env_->isolate(), std::move(backing_store_));
106106
}
107107

108108
} // namespace node

‎src/allocated_buffer.h‎

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,31 +6,43 @@
66
#include "base_object.h"
77
#include "uv.h"
88
#include "v8.h"
9+
#include "env.h"
910

1011
namespace node {
1112

1213
class Environment;
1314

15+
// Disables zero-filling for ArrayBuffer allocations in this scope. This is
16+
// similar to how we implement Buffer.allocUnsafe() in JS land.
17+
class NoArrayBufferZeroFillScope{
18+
public:
19+
inline explicit NoArrayBufferZeroFillScope(IsolateData* isolate_data);
20+
inline ~NoArrayBufferZeroFillScope();
21+
22+
private:
23+
NodeArrayBufferAllocator* node_allocator_;
24+
25+
friend class Environment;
26+
};
27+
1428
// A unique-pointer-ish object that is compatible with the JS engine's
1529
// ArrayBuffer::Allocator.
30+
// TODO(addaleax): We may want to start phasing this out as it's only a
31+
// thin wrapper around v8::BackingStore at this point
1632
struct AllocatedBuffer {
1733
public:
18-
enum AllocateManagedFlags {
19-
ALLOCATE_MANAGED_FLAG_NONE,
20-
ALLOCATE_MANAGED_UNCHECKED
21-
};
22-
2334
// Utilities that allocate memory using the Isolate's ArrayBuffer::Allocator.
2435
// In particular, using AllocateManaged() will provide a RAII-style object
2536
// with easy conversion to `Buffer` and `ArrayBuffer` objects.
26-
inline static AllocatedBuffer AllocateManaged(
27-
Environment* env,
28-
size_t size,
29-
int flags = ALLOCATE_MANAGED_FLAG_NONE);
30-
31-
explicit inline AllocatedBuffer(Environment* env = nullptr);
32-
inline AllocatedBuffer(Environment* env, uv_buf_t buf);
33-
inline ~AllocatedBuffer();
37+
inline static AllocatedBuffer AllocateManaged(Environment* env, size_t size);
38+
39+
AllocatedBuffer() = default;
40+
inline AllocatedBuffer(
41+
Environment* env, std::unique_ptr<v8::BackingStore> bs);
42+
// For this constructor variant, `buffer` *must* come from an earlier call
43+
// to .release
44+
inline AllocatedBuffer(Environment* env, uv_buf_t buffer);
45+
3446
inline void Resize(size_t len);
3547

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

45-
inline AllocatedBuffer(AllocatedBuffer&& other);
46-
inline AllocatedBuffer& operator=(AllocatedBuffer&& other);
57+
AllocatedBuffer(AllocatedBuffer&& other) = default;
58+
AllocatedBuffer& operator=(AllocatedBuffer&& other) = default;
4759
AllocatedBuffer(const AllocatedBuffer& other) = delete;
4860
AllocatedBuffer& operator=(const AllocatedBuffer& other) = delete;
4961

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

5666
friend class Environment;
5767
};

‎src/env-inl.h‎

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -867,29 +867,9 @@ inline IsolateData* Environment::isolate_data() const {
867867
return isolate_data_;
868868
}
869869

870-
inline char* Environment::AllocateUnchecked(size_t size) {
871-
return static_cast<char*>(
872-
isolate_data()->allocator()->AllocateUninitialized(size));
873-
}
874-
875-
inline char* Environment::Allocate(size_t size) {
876-
char* ret = AllocateUnchecked(size);
877-
CHECK_NE(ret, nullptr);
878-
return ret;
879-
}
880-
881-
inline void Environment::Free(char* data, size_t size) {
882-
if (data != nullptr)
883-
isolate_data()->allocator()->Free(data, size);
884-
}
885-
886-
// It's a bit awkward to define this Buffer::New() overload here, but it
887-
// avoids a circular dependency with node_internals.h.
888-
namespace Buffer {
889-
v8::MaybeLocal<v8::Object> New(Environment* env,
890-
char* data,
891-
size_t length,
892-
bool uses_malloc);
870+
std::unordered_map<char*, std::unique_ptr<v8::BackingStore>>*
871+
Environment::released_allocated_buffers() {
872+
return &released_allocated_buffers_;
893873
}
894874

895875
inline void Environment::ThrowError(const char* errmsg) {

‎src/env.cc‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1104,25 +1104,6 @@ void Environment::MemoryInfo(MemoryTracker* tracker) const {
11041104
// node, we shift its sizeof() size out of the Environment node.
11051105
}
11061106

1107-
char* Environment::Reallocate(char* data, size_t old_size, size_t size) {
1108-
if (old_size == size) return data;
1109-
// If we know that the allocator is our ArrayBufferAllocator, we can let
1110-
// if reallocate directly.
1111-
if (isolate_data()->uses_node_allocator()) {
1112-
return static_cast<char*>(
1113-
isolate_data()->node_allocator()->Reallocate(data, old_size, size));
1114-
}
1115-
// Generic allocators do not provide a reallocation method; we need to
1116-
// allocate a new chunk of memory and copy the data over.
1117-
char* new_data = AllocateUnchecked(size);
1118-
if (new_data == nullptr) return nullptr;
1119-
memcpy(new_data, data, std::min(size, old_size));
1120-
if (size > old_size)
1121-
memset(new_data + old_size, 0, size - old_size);
1122-
Free(data, old_size);
1123-
return new_data;
1124-
}
1125-
11261107
void Environment::RunWeakRefCleanup() {
11271108
isolate()->ClearKeptObjects();
11281109
}

‎src/env.h‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -926,11 +926,6 @@ class Environment : public MemoryRetainer {
926926

927927
inline IsolateData* isolate_data() const;
928928

929-
inline char* Allocate(size_t size);
930-
inline char* AllocateUnchecked(size_t size);
931-
char* Reallocate(char* data, size_t old_size, size_t size);
932-
inline void Free(char* data, size_t size);
933-
934929
inline bool printed_error() const;
935930
inline void set_printed_error(bool value);
936931

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

1216+
inline std::unordered_map<char*, std::unique_ptr<v8::BackingStore>>*
1217+
released_allocated_buffers();
1218+
12211219
private:
12221220
inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
12231221
const char* errmsg);
@@ -1378,6 +1376,11 @@ class Environment : public MemoryRetainer {
13781376
// We should probably find a way to just use plain `v8::String`s created from
13791377
// the source passed to LoadEnvironment() directly instead.
13801378
std::unique_ptr<v8::String::Value> main_utf16_;
1379+
1380+
// Used by AllocatedBuffer::release() to keep track of the BackingStore for
1381+
// a given pointer.
1382+
std::unordered_map<char*, std::unique_ptr<v8::BackingStore>>
1383+
released_allocated_buffers_;
13811384
};
13821385

13831386
} // namespace node

0 commit comments

Comments
 (0)