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: refacor MallocedBuffer to it's usage scope #23641

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.4',
'v8_embedder_string': '-node.5',

# Enable disassembler for `--print-code` v8 options
'v8_enable_disassembler': 1,
Expand Down
24 changes: 0 additions & 24 deletions deps/v8/include/v8.h
Original file line number Diff line number Diff line change
Expand Up @@ -4362,13 +4362,6 @@ class V8_EXPORT WasmCompiledModule : public Object {
public:
typedef std::pair<std::unique_ptr<const uint8_t[]>, size_t> SerializedModule;

// The COMMA macro allows us to use ',' inside of the V8_DEPRECATED macro.
#define COMMA ,
V8_DEPRECATED(
"Use BufferReference.",
typedef std::pair<const uint8_t * COMMA size_t> CallerOwnedBuffer);
#undef COMMA

/**
* A unowned reference to a byte buffer.
*/
Expand All @@ -4377,12 +4370,6 @@ class V8_EXPORT WasmCompiledModule : public Object {
size_t size;
BufferReference(const uint8_t* start, size_t size)
: start(start), size(size) {}
// Temporarily allow conversion to and from CallerOwnedBuffer.
V8_DEPRECATED(
"Use BufferReference directly.",
inline BufferReference(CallerOwnedBuffer)); // NOLINT(runtime/explicit)
V8_DEPRECATED("Use BufferReference directly.",
inline operator CallerOwnedBuffer());
};

/**
Expand Down Expand Up @@ -4429,8 +4416,6 @@ class V8_EXPORT WasmCompiledModule : public Object {
* Get the wasm-encoded bytes that were used to compile this module.
*/
BufferReference GetWasmWireBytesRef();
V8_DEPRECATED("Use GetWasmWireBytesRef version.",
Local<String> GetWasmWireBytes());

/**
* Serialize the compiled module. The serialized data does not include the
Expand Down Expand Up @@ -4463,15 +4448,6 @@ class V8_EXPORT WasmCompiledModule : public Object {
static void CheckCast(Value* obj);
};

// TODO(clemensh): Remove after M70 branch.
WasmCompiledModule::BufferReference::BufferReference(
WasmCompiledModule::CallerOwnedBuffer buf)
: BufferReference(buf.first, buf.second) {}
WasmCompiledModule::BufferReference::
operator WasmCompiledModule::CallerOwnedBuffer() {
return {start, size};
}

/**
* The V8 interface for WebAssembly streaming compilation. When streaming
* compilation is initiated, V8 passes a {WasmStreaming} object to the embedder
Expand Down
8 changes: 0 additions & 8 deletions deps/v8/src/api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7382,14 +7382,6 @@ WasmCompiledModule::BufferReference WasmCompiledModule::GetWasmWireBytesRef() {
return {bytes_vec.start(), bytes_vec.size()};
}

Local<String> WasmCompiledModule::GetWasmWireBytes() {
BufferReference ref = GetWasmWireBytesRef();
CHECK_LE(ref.size, String::kMaxLength);
return String::NewFromOneByte(GetIsolate(), ref.start, NewStringType::kNormal,
static_cast<int>(ref.size))
.ToLocalChecked();
}

WasmCompiledModule::TransferrableModule
WasmCompiledModule::GetTransferrableModule() {
if (i::FLAG_wasm_shared_code) {
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/bootstrap/loaders.js
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@
NativeModule.isInternal = function(id) {
return id.startsWith('internal/') ||
(id === 'worker_threads' &&
!process.binding('config').experimentalWorker);
!internalBinding('options').getOptions('--experimental-worker'));
};
}

Expand Down
2 changes: 1 addition & 1 deletion lib/internal/repl/await.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ function processTopLevelAwait(src) {
let root;
try {
root = acorn.parse(wrapped, { ecmaVersion: 10 });
} catch (err) {
} catch {
return null;
}
const body = root.body[0].expression.callee.body;
Expand Down
2 changes: 1 addition & 1 deletion lib/querystring.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ function unescapeBuffer(s, decodeSpaces) {
function qsUnescape(s, decodeSpaces) {
try {
return decodeURIComponent(s);
} catch (e) {
} catch {
return QueryString.unescapeBuffer(s, decodeSpaces).toString();
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -1485,7 +1485,7 @@ function defineDefaultCommands(repl) {
this.outputStream.write('Failed to load: ' + file +
' is not a valid file\n');
}
} catch (e) {
} catch {
this.outputStream.write('Failed to load: ' + file + '\n');
}
this.displayPrompt();
Expand Down
7 changes: 0 additions & 7 deletions src/memory_tracker-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,6 @@ void MemoryTracker::TrackField(const char* edge_name,
graph_->AddEdge(CurrentNode(), graph_->V8Node(value), edge_name);
}

template <typename T>
void MemoryTracker::TrackField(const char* edge_name,
const MallocedBuffer<T>& value,
const char* node_name) {
TrackFieldWithSize(edge_name, value.size, "MallocedBuffer");
}

void MemoryTracker::TrackField(const char* name,
const uv_buf_t& value,
const char* node_name) {
Expand Down
4 changes: 0 additions & 4 deletions src/memory_tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,6 @@ class MemoryTracker {
inline void TrackField(const char* edge_name,
const v8::Local<T>& value,
const char* node_name = nullptr);
template <typename T>
inline void TrackField(const char* edge_name,
const MallocedBuffer<T>& value,
const char* node_name = nullptr);
inline void TrackField(const char* edge_name,
const uv_buf_t& value,
const char* node_name = nullptr);
Expand Down
15 changes: 8 additions & 7 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4197,9 +4197,10 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
Buffer::Length(args[0]),
0));

MallocedBuffer<char> data(DH_size(diffieHellman->dh_.get()));
size_t dh_size = DH_size(diffieHellman->dh_.get());
malloced_unique_ptr<char> data = make_malloced_unique<char>(dh_size);

int size = DH_compute_key(reinterpret_cast<unsigned char*>(data.data),
int size = DH_compute_key(reinterpret_cast<unsigned char*>(data.get()),
key.get(),
diffieHellman->dh_.get());

Expand Down Expand Up @@ -4234,14 +4235,14 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
// DH_compute_key returns number of bytes in a remainder of exponent, which
// may have less bytes than a prime number. Therefore add 0-padding to the
// allocated buffer.
if (static_cast<size_t>(size) != data.size) {
CHECK_GT(data.size, static_cast<size_t>(size));
memmove(data.data + data.size - size, data.data, size);
memset(data.data, 0, data.size - size);
if (static_cast<size_t>(size) != dh_size) {
CHECK_GT(dh_size, static_cast<size_t>(size));
memmove(data.get() + dh_size - size, data.get(), size);
memset(data.get(), 0, dh_size - size);
}

args.GetReturnValue().Set(
Buffer::New(env->isolate(), data.release(), data.size).ToLocalChecked());
Buffer::New(env->isolate(), data.release(), dh_size).ToLocalChecked());
}

void DiffieHellman::SetKey(const v8::FunctionCallbackInfo<Value>& args,
Expand Down
2 changes: 1 addition & 1 deletion src/node_messaging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ Maybe<bool> Message::Serialize(Environment* env,
}

void Message::MemoryInfo(MemoryTracker* tracker) const {
tracker->TrackField("array_buffer_contents", array_buffer_contents_);
tracker->TrackFieldWithSize("array_buffer_contents", array_buffer_contents_.size(), "MallocedBuffer");
tracker->TrackFieldWithSize("shared_array_buffers",
shared_array_buffers_.size() * sizeof(shared_array_buffers_[0]));
tracker->TrackField("message_ports", message_ports_);
Expand Down
31 changes: 31 additions & 0 deletions src/node_messaging.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,37 @@
namespace node {
namespace worker {

// Simple RAII wrapper for contiguous data that uses malloc()/free().
template <typename T>
struct MallocedBuffer {
T* data;
size_t size;

T* release() {
T* ret = data;
data = nullptr;
return ret;
}

inline bool is_empty() const { return data == nullptr; }

MallocedBuffer() : data(nullptr) {}
explicit MallocedBuffer(size_t size) : data(Malloc<T>(size)), size(size) {}
MallocedBuffer(T* data, size_t size) : data(data), size(size) {}
MallocedBuffer(MallocedBuffer&& other) : data(other.data), size(other.size) {
other.data = nullptr;
}
MallocedBuffer& operator=(MallocedBuffer&& other) {
this->~MallocedBuffer();
return *new(this) MallocedBuffer(std::move(other));
}
~MallocedBuffer() {
free(data);
}
MallocedBuffer(const MallocedBuffer&) = delete;
MallocedBuffer& operator=(const MallocedBuffer&) = delete;
};

class MessagePortData;
class MessagePort;

Expand Down
15 changes: 9 additions & 6 deletions src/node_platform.cc
Original file line number Diff line number Diff line change
Expand Up @@ -167,17 +167,20 @@ class WorkerThreadsTaskRunner::DelayedTaskScheduler {
};

WorkerThreadsTaskRunner::WorkerThreadsTaskRunner(int thread_pool_size) {
Mutex::ScopedLock lock(platform_workers_mutex_);
pending_platform_workers_ = thread_pool_size;
Mutex platform_workers_mutex;
ConditionVariable platform_workers_ready;

Mutex::ScopedLock lock(platform_workers_mutex);
int pending_platform_workers = thread_pool_size;

delayed_task_scheduler_.reset(
new DelayedTaskScheduler(&pending_worker_tasks_));
threads_.push_back(delayed_task_scheduler_->Start());

for (int i = 0; i < thread_pool_size; i++) {
PlatformWorkerData* worker_data = new PlatformWorkerData{
&pending_worker_tasks_, &platform_workers_mutex_,
&platform_workers_ready_, &pending_platform_workers_, i
&pending_worker_tasks_, &platform_workers_mutex,
&platform_workers_ready, &pending_platform_workers, i
};
std::unique_ptr<uv_thread_t> t { new uv_thread_t() };
if (uv_thread_create(t.get(), PlatformWorkerThread,
Expand All @@ -189,8 +192,8 @@ WorkerThreadsTaskRunner::WorkerThreadsTaskRunner(int thread_pool_size) {

// Wait for platform workers to initialize before continuing with the
// bootstrap.
while (pending_platform_workers_ > 0) {
platform_workers_ready_.Wait(lock);
while (pending_platform_workers > 0) {
platform_workers_ready.Wait(lock);
}
}

Expand Down
4 changes: 0 additions & 4 deletions src/node_platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,6 @@ class WorkerThreadsTaskRunner {
std::unique_ptr<DelayedTaskScheduler> delayed_task_scheduler_;

std::vector<std::unique_ptr<uv_thread_t>> threads_;

Mutex platform_workers_mutex_;
ConditionVariable platform_workers_ready_;
int pending_platform_workers_;
};

class NodePlatform : public MultiIsolatePlatform {
Expand Down
47 changes: 16 additions & 31 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -427,37 +427,6 @@ struct OnScopeLeave {
~OnScopeLeave() { fn_(); }
};

// Simple RAII wrapper for contiguous data that uses malloc()/free().
template <typename T>
struct MallocedBuffer {
T* data;
size_t size;

T* release() {
T* ret = data;
data = nullptr;
return ret;
}

inline bool is_empty() const { return data == nullptr; }

MallocedBuffer() : data(nullptr) {}
explicit MallocedBuffer(size_t size) : data(Malloc<T>(size)), size(size) {}
MallocedBuffer(T* data, size_t size) : data(data), size(size) {}
MallocedBuffer(MallocedBuffer&& other) : data(other.data), size(other.size) {
other.data = nullptr;
}
MallocedBuffer& operator=(MallocedBuffer&& other) {
this->~MallocedBuffer();
return *new(this) MallocedBuffer(std::move(other));
}
~MallocedBuffer() {
free(data);
}
MallocedBuffer(const MallocedBuffer&) = delete;
MallocedBuffer& operator=(const MallocedBuffer&) = delete;
};

// Test whether some value can be called with ().
template <typename T, typename = void>
struct is_callable : std::is_function<T> { };
Expand Down Expand Up @@ -487,6 +456,22 @@ template <typename T, typename U>
inline v8::MaybeLocal<v8::Value> ToV8Value(v8::Local<v8::Context> context,
const std::unordered_map<T, U>& map);

// Helper for `malloced_unique_ptr`
template<typename T>
struct Free {
void operator()(T* ptr) const { free(ptr); }
};

// Specialization of `std::unique_ptr` that used `Malloc<t>`
template<typename T>
using malloced_unique_ptr = std::unique_ptr<T, Free<T>>;

// Factory of `malloced_unique_ptr`
template<typename T>
malloced_unique_ptr<T> make_malloced_unique(size_t number_of_t) {
return malloced_unique_ptr<T>(Malloc<T>(number_of_t));
}

} // namespace node

#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
Expand Down
Loading