Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 69dac4b

Browse files
addaleaxMylesBorins
authored andcommittedJan 10, 2020
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 f080239 commit 69dac4b

11 files changed

+44
-21
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/node_binding.cc

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

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

159159
if (rc != 0) {

‎src/node_env_var.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ Local<Array> RealEnvStore::Enumerate(Isolate* isolate) const {
132132
uv_env_item_t* items;
133133
int count;
134134

135-
OnScopeLeave cleanup([&]() { uv_os_free_environ(items, count); });
135+
auto cleanup = OnScopeLeave([&]() { uv_os_free_environ(items, count); });
136136
CHECK_EQ(uv_os_environ(&items, &count), 0);
137137

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

‎src/node_http_parser_impl.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -606,7 +606,7 @@ class Parser : public AsyncWrap, public StreamListener {
606606
// Once we’re done here, either indicate that the HTTP parser buffer
607607
// is free for re-use, or free() the data if it didn’t come from there
608608
// in the first place.
609-
OnScopeLeave on_scope_leave([&]() {
609+
auto on_scope_leave = OnScopeLeave([&]() {
610610
if (buf.base == env()->http_parser_buffer())
611611
env()->set_http_parser_buffer_in_use(false);
612612
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
@@ -831,7 +831,7 @@ void GetOptions(const FunctionCallbackInfo<Value>& args) {
831831
per_process::cli_options->per_isolate = env->isolate_data()->options();
832832
auto original_per_env = per_process::cli_options->per_isolate->per_env;
833833
per_process::cli_options->per_isolate->per_env = env->options();
834-
OnScopeLeave on_scope_leave([&]() {
834+
auto on_scope_leave = OnScopeLeave([&]() {
835835
per_process::cli_options->per_isolate->per_env = original_per_env;
836836
per_process::cli_options->per_isolate = original_per_isolate;
837837
});

‎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
@@ -355,7 +355,7 @@ static void DebugProcess(const FunctionCallbackInfo<Value>& args) {
355355
LPTHREAD_START_ROUTINE* handler = nullptr;
356356
DWORD pid = 0;
357357

358-
OnScopeLeave cleanup([&]() {
358+
auto cleanup = OnScopeLeave([&]() {
359359
if (process != nullptr) CloseHandle(process);
360360
if (thread != nullptr) CloseHandle(thread);
361361
if (handler != nullptr) UnmapViewOfFile(handler);

‎src/node_worker.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ void Worker::Run() {
182182
SealHandleScope outer_seal(isolate_);
183183

184184
DeleteFnPtr<Environment, FreeEnvironment> env_;
185-
OnScopeLeave cleanup_env([&]() {
185+
auto cleanup_env = OnScopeLeave([&]() {
186186
if (!env_) return;
187187
env_->set_can_call_into_js(false);
188188
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
@@ -489,14 +495,37 @@ class BufferValue : public MaybeStackBuffer<char> {
489495
// silence a compiler warning about that.
490496
template <typename T> inline void USE(T&&) {}
491497

492-
// Run a function when exiting the current scope.
493-
struct OnScopeLeave {
494-
std::function<void()> fn_;
498+
template <typename Fn>
499+
struct OnScopeLeaveImpl {
500+
Fn fn_;
501+
bool active_;
502+
503+
explicit OnScopeLeaveImpl(Fn&& fn) : fn_(std::move(fn)), active_(true) {}
504+
~OnScopeLeaveImpl() { if (active_) fn_(); }
495505

496-
explicit OnScopeLeave(std::function<void()> fn) : fn_(std::move(fn)) {}
497-
~OnScopeLeave() { fn_(); }
506+
OnScopeLeaveImpl(const OnScopeLeaveImpl& other) = delete;
507+
OnScopeLeaveImpl& operator=(const OnScopeLeaveImpl& other) = delete;
508+
OnScopeLeaveImpl(OnScopeLeaveImpl&& other)
509+
: fn_(std::move(other.fn_)), active_(other.active_) {
510+
other.active_ = false;
511+
}
512+
OnScopeLeaveImpl& operator=(OnScopeLeaveImpl&& other) {
513+
if (this == &other) return *this;
514+
this->~OnScopeLeave();
515+
new (this)OnScopeLeaveImpl(std::move(other));
516+
return *this;
517+
}
498518
};
499519

520+
// Run a function when exiting the current scope. Used like this:
521+
// auto on_scope_leave = OnScopeLeave([&] {
522+
// // ... run some code ...
523+
// });
524+
template <typename Fn>
525+
inline MUST_USE_RESULT OnScopeLeaveImpl<Fn> OnScopeLeave(Fn&& fn) {
526+
return OnScopeLeaveImpl<Fn>{std::move(fn)};
527+
}
528+
500529
// Simple RAII wrapper for contiguous data that uses malloc()/free().
501530
template <typename T>
502531
struct MallocedBuffer {
@@ -674,12 +703,6 @@ constexpr T RoundUp(T a, T b) {
674703
return a % b != 0 ? a + b - (a % b) : a;
675704
}
676705

677-
#ifdef __GNUC__
678-
#define MUST_USE_RESULT __attribute__((warn_unused_result))
679-
#else
680-
#define MUST_USE_RESULT
681-
#endif
682-
683706
class SlicedArguments : public MaybeStackBuffer<v8::Local<v8::Value>> {
684707
public:
685708
inline explicit SlicedArguments(

0 commit comments

Comments
 (0)