src: prevent extra copies of TimerWrap::TimerCb

I noticed that we were taking `TimerCb` as a `const&` and then copying
that into the member. This is completely fine when the constructor is
called with an lvalue. However, when called with an rvalue, we can allow
the `std::function` to be moved into the member instead of falling back
to a copy, so I changed the constructors to take in universal
references. Also, `std::function` constructors can take in multiple
arguments, so I further modified the constructors to use variadic
templates.

Signed-off-by: Darshan Sen <darshan.sen@postman.com>

PR-URL: https://github.com/nodejs/node/pull/40665
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This commit is contained in:
Darshan Sen authored and GitHub committed 2021-11-12 21:47:42 +00:00
1 parent 2037ee85a2
commit 2d368da19f
5 files changed
+42 -20

No files matched your search

+1
View File
@@ -642,6 +642,7 @@
'src/tracing/trace_event_common.h',
'src/tracing/traced_value.h',
'src/timer_wrap.h',
'src/timer_wrap-inl.h',
'src/tty_wrap.h',
'src/udp_wrap.h',
'src/util.h',
+1 -1
View File
@@ -15,7 +15,7 @@
#include "node_process-inl.h"
#include "node_url.h"
#include "util-inl.h"
#include "timer_wrap.h"
#include "timer_wrap-inl.h"
#include "v8-inspector.h"
#include "v8-platform.h"
+32
View File
@@ -0,0 +1,32 @@
#ifndef SRC_TIMER_WRAP_INL_H_
#define SRC_TIMER_WRAP_INL_H_
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
#include "timer_wrap.h"
#include <utility>
#include "env.h"
#include "uv.h"
namespace node {
template <typename... Args>
inline TimerWrap::TimerWrap(Environment* env, Args&&... args)
: env_(env), fn_(std::forward<Args>(args)...) {
uv_timer_init(env->event_loop(), &timer_);
timer_.data = this;
}
template <typename... Args>
inline TimerWrapHandle::TimerWrapHandle(Environment* env, Args&&... args) {
timer_ = new TimerWrap(env, std::forward<Args>(args)...);
env->AddCleanupHook(CleanupHook, this);
}
} // namespace node
#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
#endif // SRC_TIMER_WRAP_INL_H_
+3 -15
View File
@@ -1,17 +1,12 @@
#include "timer_wrap.h" // NOLINT(build/include_inline)
#include "timer_wrap-inl.h"
#include "env-inl.h"
#include "memory_tracker-inl.h"
#include "timer_wrap.h"
#include "uv.h"
namespace node {
TimerWrap::TimerWrap(Environment* env, const TimerCb& fn)
: env_(env),
fn_(fn) {
uv_timer_init(env->event_loop(), &timer_);
timer_.data = this;
}
void TimerWrap::Stop() {
if (timer_.data == nullptr) return;
uv_timer_stop(&timer_);
@@ -48,13 +43,6 @@ void TimerWrap::OnTimeout(uv_timer_t* timer) {
t->fn_();
}
TimerWrapHandle::TimerWrapHandle(
Environment* env,
const TimerWrap::TimerCb& fn) {
timer_ = new TimerWrap(env, fn);
env->AddCleanupHook(CleanupHook, this);
}
void TimerWrapHandle::Stop() {
if (timer_ != nullptr)
return timer_->Stop();
+5 -4
View File
@@ -16,7 +16,9 @@ class TimerWrap final : public MemoryRetainer {
public:
using TimerCb = std::function<void()>;
TimerWrap(Environment* env, const TimerCb& fn);
template <typename... Args>
explicit inline TimerWrap(Environment* env, Args&&... args);
TimerWrap(const TimerWrap&) = delete;
inline Environment* env() const { return env_; }
@@ -50,9 +52,8 @@ class TimerWrap final : public MemoryRetainer {
class TimerWrapHandle : public MemoryRetainer {
public:
TimerWrapHandle(
Environment* env,
const TimerWrap::TimerCb& fn);
template <typename... Args>
explicit inline TimerWrapHandle(Environment* env, Args&&... args);
TimerWrapHandle(const TimerWrapHandle&) = delete;