Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 3be49d6

Browse filesBrowse files
RaisinTentargos
RaisinTen
authored and
targos
committed
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: #40665 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent a5a1691 commit 3be49d6
Copy full SHA for 3be49d6

File tree

Expand file treeCollapse file tree

5 files changed

+42
-20
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+42
-20
lines changed
Open diff view settings
Collapse file

‎node.gyp‎

Copy file name to clipboardExpand all lines: node.gyp
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -642,6 +642,7 @@
642642
'src/tracing/trace_event_common.h',
643643
'src/tracing/traced_value.h',
644644
'src/timer_wrap.h',
645+
'src/timer_wrap-inl.h',
645646
'src/tty_wrap.h',
646647
'src/udp_wrap.h',
647648
'src/util.h',
Collapse file

‎src/inspector_agent.cc‎

Copy file name to clipboardExpand all lines: src/inspector_agent.cc
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
#include "node_process-inl.h"
1616
#include "node_url.h"
1717
#include "util-inl.h"
18-
#include "timer_wrap.h"
18+
#include "timer_wrap-inl.h"
1919
#include "v8-inspector.h"
2020
#include "v8-platform.h"
2121

Collapse file

‎src/timer_wrap-inl.h‎

Copy file name to clipboard
+32Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
#ifndef SRC_TIMER_WRAP_INL_H_
2+
#define SRC_TIMER_WRAP_INL_H_
3+
4+
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
5+
6+
#include "timer_wrap.h"
7+
8+
#include <utility>
9+
10+
#include "env.h"
11+
#include "uv.h"
12+
13+
namespace node {
14+
15+
template <typename... Args>
16+
inline TimerWrap::TimerWrap(Environment* env, Args&&... args)
17+
: env_(env), fn_(std::forward<Args>(args)...) {
18+
uv_timer_init(env->event_loop(), &timer_);
19+
timer_.data = this;
20+
}
21+
22+
template <typename... Args>
23+
inline TimerWrapHandle::TimerWrapHandle(Environment* env, Args&&... args) {
24+
timer_ = new TimerWrap(env, std::forward<Args>(args)...);
25+
env->AddCleanupHook(CleanupHook, this);
26+
}
27+
28+
} // namespace node
29+
30+
#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
31+
32+
#endif // SRC_TIMER_WRAP_INL_H_
Collapse file

‎src/timer_wrap.cc‎

Copy file name to clipboardExpand all lines: src/timer_wrap.cc
+3-15Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,12 @@
1+
#include "timer_wrap.h" // NOLINT(build/include_inline)
2+
#include "timer_wrap-inl.h"
3+
14
#include "env-inl.h"
25
#include "memory_tracker-inl.h"
3-
#include "timer_wrap.h"
46
#include "uv.h"
57

68
namespace node {
79

8-
TimerWrap::TimerWrap(Environment* env, const TimerCb& fn)
9-
: env_(env),
10-
fn_(fn) {
11-
uv_timer_init(env->event_loop(), &timer_);
12-
timer_.data = this;
13-
}
14-
1510
void TimerWrap::Stop() {
1611
if (timer_.data == nullptr) return;
1712
uv_timer_stop(&timer_);
@@ -48,13 +43,6 @@ void TimerWrap::OnTimeout(uv_timer_t* timer) {
4843
t->fn_();
4944
}
5045

51-
TimerWrapHandle::TimerWrapHandle(
52-
Environment* env,
53-
const TimerWrap::TimerCb& fn) {
54-
timer_ = new TimerWrap(env, fn);
55-
env->AddCleanupHook(CleanupHook, this);
56-
}
57-
5846
void TimerWrapHandle::Stop() {
5947
if (timer_ != nullptr)
6048
return timer_->Stop();
Collapse file

‎src/timer_wrap.h‎

Copy file name to clipboardExpand all lines: src/timer_wrap.h
+5-4Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@ class TimerWrap final : public MemoryRetainer {
1616
public:
1717
using TimerCb = std::function<void()>;
1818

19-
TimerWrap(Environment* env, const TimerCb& fn);
19+
template <typename... Args>
20+
explicit inline TimerWrap(Environment* env, Args&&... args);
21+
2022
TimerWrap(const TimerWrap&) = delete;
2123

2224
inline Environment* env() const { return env_; }
@@ -50,9 +52,8 @@ class TimerWrap final : public MemoryRetainer {
5052

5153
class TimerWrapHandle : public MemoryRetainer {
5254
public:
53-
TimerWrapHandle(
54-
Environment* env,
55-
const TimerWrap::TimerCb& fn);
55+
template <typename... Args>
56+
explicit inline TimerWrapHandle(Environment* env, Args&&... args);
5657

5758
TimerWrapHandle(const TimerWrapHandle&) = delete;
5859

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.