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>

5 files changed, 42 insertions, 20 deletions

diff --git a/node.gyp b/node.gyp
index d2337823a8..068a647e41 100644
--- a/node.gyp
+++ b/node.gyp
@@ -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',
diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc
index c4a3322c6d..fd9f514b9b 100644
--- a/src/inspector_agent.cc
+++ b/src/inspector_agent.cc
@@ -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"
 
diff --git a/src/timer_wrap-inl.h b/src/timer_wrap-inl.h
new file mode 100644
index 0000000000..d60ea15ade
--- /dev/null
+++ b/src/timer_wrap-inl.h
@@ -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_
diff --git a/src/timer_wrap.cc b/src/timer_wrap.cc
index 6660b8c958..2b4457df81 100644
--- a/src/timer_wrap.cc
+++ b/src/timer_wrap.cc
@@ -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();
diff --git a/src/timer_wrap.h b/src/timer_wrap.h
index dbc23b442b..ac8f00f0d4 100644
--- a/src/timer_wrap.h
+++ b/src/timer_wrap.h
@@ -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;