Skip to content

Commit 058a8d5

Browse files
addaleaxMylesBorins
authored andcommitted
src: do not use std::function for OnScopeLeave
Using `std::function` adds an extra layer of indirection, and in particular, heap allocations that are not necessary in our use case here. PR-URL: #30134 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 31a3b72 commit 058a8d5

12 files changed

+45
-22
lines changed

src/api/callback.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ void InternalCallbackScope::Close() {
105105

106106
if (!env_->can_call_into_js()) return;
107107

108-
OnScopeLeave weakref_cleanup([&]() { env_->RunWeakRefCleanup(); });
108+
auto weakref_cleanup = OnScopeLeave([&]() { env_->RunWeakRefCleanup(); });
109109

110110
if (!tick_info->has_tick_scheduled()) {
111111
MicrotasksScope::PerformCheckpoint(env_->isolate());

src/large_pages/node_large_page.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ MoveTextRegionToLargePages(const text_region& r) {
333333
PrintSystemError(errno);
334334
return -1;
335335
}
336-
OnScopeLeave munmap_on_return([nmem, size]() {
336+
auto munmap_on_return = OnScopeLeave([nmem, size]() {
337337
if (-1 == munmap(nmem, size)) PrintSystemError(errno);
338338
});
339339

src/node_binding.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ void* wrapped_dlopen(const char* filename, int flags) {
152152
Mutex::ScopedLock lock(dlhandles_mutex);
153153

154154
uv_fs_t req;
155-
OnScopeLeave cleanup([&]() { uv_fs_req_cleanup(&req); });
155+
auto cleanup = OnScopeLeave([&]() { uv_fs_req_cleanup(&req); });
156156
int rc = uv_fs_stat(nullptr, &req, filename, nullptr);
157157

158158
if (rc != 0) {

src/node_env_var.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ Local<Array> RealEnvStore::Enumerate(Isolate* isolate) const {
149149
uv_env_item_t* items;
150150
int count;
151151

152-
OnScopeLeave cleanup([&]() { uv_os_free_environ(items, count); });
152+
auto cleanup = OnScopeLeave([&]() { uv_os_free_environ(items, count); });
153153
CHECK_EQ(uv_os_environ(&items, &count), 0);
154154

155155
MaybeStackBuffer<Local<Value>, 256> env_v(count);

src/node_http_parser.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,7 @@ class Parser : public AsyncWrap, public StreamListener {
586586
// Once we’re done here, either indicate that the HTTP parser buffer
587587
// is free for re-use, or free() the data if it didn’t come from there
588588
// in the first place.
589-
OnScopeLeave on_scope_leave([&]() {
589+
auto on_scope_leave = OnScopeLeave([&]() {
590590
if (buf.base == env()->http_parser_buffer())
591591
env()->set_http_parser_buffer_in_use(false);
592592
else

src/node_i18n.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ class ConverterObject : public BaseObject, Converter {
217217
result.AllocateSufficientStorage(limit);
218218

219219
UBool flush = (flags & CONVERTER_FLAGS_FLUSH) == CONVERTER_FLAGS_FLUSH;
220-
OnScopeLeave cleanup([&]() {
220+
auto cleanup = OnScopeLeave([&]() {
221221
if (flush) {
222222
// Reset the converter state.
223223
converter->bomSeen_ = false;

src/node_options.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -809,7 +809,7 @@ void GetOptions(const FunctionCallbackInfo<Value>& args) {
809809
per_process::cli_options->per_isolate = env->isolate_data()->options();
810810
auto original_per_env = per_process::cli_options->per_isolate->per_env;
811811
per_process::cli_options->per_isolate->per_env = env->options();
812-
OnScopeLeave on_scope_leave([&]() {
812+
auto on_scope_leave = OnScopeLeave([&]() {
813813
per_process::cli_options->per_isolate->per_env = original_per_env;
814814
per_process::cli_options->per_isolate = original_per_isolate;
815815
});

src/node_os.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ static void GetUserInfo(const FunctionCallbackInfo<Value>& args) {
302302
return args.GetReturnValue().SetUndefined();
303303
}
304304

305-
OnScopeLeave free_passwd([&]() { uv_os_free_passwd(&pwd); });
305+
auto free_passwd = OnScopeLeave([&]() { uv_os_free_passwd(&pwd); });
306306

307307
Local<Value> error;
308308

src/node_process_methods.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ static void DebugProcess(const FunctionCallbackInfo<Value>& args) {
362362
LPTHREAD_START_ROUTINE* handler = nullptr;
363363
DWORD pid = 0;
364364

365-
OnScopeLeave cleanup([&]() {
365+
auto cleanup = OnScopeLeave([&]() {
366366
if (process != nullptr) CloseHandle(process);
367367
if (thread != nullptr) CloseHandle(thread);
368368
if (handler != nullptr) UnmapViewOfFile(handler);

src/node_worker.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ void Worker::Run() {
242242
SealHandleScope outer_seal(isolate_);
243243

244244
DeleteFnPtr<Environment, FreeEnvironment> env_;
245-
OnScopeLeave cleanup_env([&]() {
245+
auto cleanup_env = OnScopeLeave([&]() {
246246
if (!env_) return;
247247
env_->set_can_call_into_js(false);
248248
Isolate::DisallowJavascriptExecutionScope disallow_js(isolate_,

src/node_zlib.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,7 @@ class CompressionStream : public AsyncWrap, public ThreadPoolWork {
386386
// v8 land!
387387
void AfterThreadPoolWork(int status) override {
388388
AllocScope alloc_scope(this);
389-
OnScopeLeave on_scope_leave([&]() { Unref(); });
389+
auto on_scope_leave = OnScopeLeave([&]() { Unref(); });
390390

391391
write_in_progress_ = false;
392392

src/util.h

+34-11
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@
4242
#include <unordered_map>
4343
#include <utility>
4444

45+
#ifdef __GNUC__
46+
#define MUST_USE_RESULT __attribute__((warn_unused_result))
47+
#else
48+
#define MUST_USE_RESULT
49+
#endif
50+
4551
namespace node {
4652

4753
// Maybe remove kPathSeparator when cpp17 is ready
@@ -494,14 +500,37 @@ class BufferValue : public MaybeStackBuffer<char> {
494500
// silence a compiler warning about that.
495501
template <typename T> inline void USE(T&&) {}
496502

497-
// Run a function when exiting the current scope.
498-
struct OnScopeLeave {
499-
std::function<void()> fn_;
503+
template <typename Fn>
504+
struct OnScopeLeaveImpl {
505+
Fn fn_;
506+
bool active_;
507+
508+
explicit OnScopeLeaveImpl(Fn&& fn) : fn_(std::move(fn)), active_(true) {}
509+
~OnScopeLeaveImpl() { if (active_) fn_(); }
500510

501-
explicit OnScopeLeave(std::function<void()> fn) : fn_(std::move(fn)) {}
502-
~OnScopeLeave() { fn_(); }
511+
OnScopeLeaveImpl(const OnScopeLeaveImpl& other) = delete;
512+
OnScopeLeaveImpl& operator=(const OnScopeLeaveImpl& other) = delete;
513+
OnScopeLeaveImpl(OnScopeLeaveImpl&& other)
514+
: fn_(std::move(other.fn_)), active_(other.active_) {
515+
other.active_ = false;
516+
}
517+
OnScopeLeaveImpl& operator=(OnScopeLeaveImpl&& other) {
518+
if (this == &other) return *this;
519+
this->~OnScopeLeave();
520+
new (this)OnScopeLeaveImpl(std::move(other));
521+
return *this;
522+
}
503523
};
504524

525+
// Run a function when exiting the current scope. Used like this:
526+
// auto on_scope_leave = OnScopeLeave([&] {
527+
// // ... run some code ...
528+
// });
529+
template <typename Fn>
530+
inline MUST_USE_RESULT OnScopeLeaveImpl<Fn> OnScopeLeave(Fn&& fn) {
531+
return OnScopeLeaveImpl<Fn>{std::move(fn)};
532+
}
533+
505534
// Simple RAII wrapper for contiguous data that uses malloc()/free().
506535
template <typename T>
507536
struct MallocedBuffer {
@@ -679,12 +708,6 @@ constexpr T RoundUp(T a, T b) {
679708
return a % b != 0 ? a + b - (a % b) : a;
680709
}
681710

682-
#ifdef __GNUC__
683-
#define MUST_USE_RESULT __attribute__((warn_unused_result))
684-
#else
685-
#define MUST_USE_RESULT
686-
#endif
687-
688711
class SlicedArguments : public MaybeStackBuffer<v8::Local<v8::Value>> {
689712
public:
690713
inline explicit SlicedArguments(

0 commit comments

Comments
 (0)