Skip to content

Commit 54a6c1f

Browse files
author
Minggang Wang
committed
Add timeout parameter for rcl_wait function
Before this patch, the timeout used for the rcl_wait function is hard coded which is 10ms. Apparently, this is not reasonable, because it imposes unnecessary workload on the CPU (We need to lock/unlock the resources shared between the main thread and the background thread frequently). Please check the detailed record in the issue. This patch adds the timeout parameter which developer can customize the value when starting a node. The default value now is 10ms, negtive value will block the thread until there is an event. So you can change your JS code like: rclnodejs.spin(node, -1); // Waits until something happens. Fix #470
1 parent dcbbdbf commit 54a6c1f

File tree

10 files changed

+207
-169
lines changed

10 files changed

+207
-169
lines changed

index.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,16 +175,17 @@ let rcl = {
175175
/**
176176
* Start to spin the node, which triggers the event loop to start to check the incoming events.
177177
* @param {Node} node - The node to be spun.
178+
* @param {number} [timeout=10] - ms to wait, block forever if negative, don't wait if 0, default is 10.
178179
* @return {undefined}
179180
*/
180-
spin(node) {
181+
spin(node, timeout = 10) {
181182
if (!(node instanceof rclnodejs.ShadowNode)) {
182183
throw new TypeError('Invalid argument.');
183184
}
184185
if (node.spinning) {
185186
throw new Error('The node is already spinning.');
186187
}
187-
node.startSpinning(this._context.handle());
188+
node.startSpinning(this._context.handle(), timeout);
188189
},
189190

190191
/**

lib/node.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@ class Node {
8686
});
8787
}
8888

89-
startSpinning(context) {
90-
this.start(context);
89+
startSpinning(context, timeout) {
90+
this.start(context, timeout);
9191
this.spinning = true;
9292
}
9393

src/executor.cpp

Lines changed: 34 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include <string>
2121

2222
#include "handle_manager.hpp"
23+
#include "rcl_bindings.hpp"
2324
#include "spdlog/spdlog.h"
2425

2526
namespace rclnodejs {
@@ -29,19 +30,21 @@ static std::exception_ptr g_exception_ptr = nullptr;
2930
Executor::Executor(HandleManager* handle_manager, Delegate* delegate)
3031
: async_(nullptr),
3132
handle_manager_(handle_manager),
32-
context_(nullptr),
33-
delegate_(delegate) {
33+
delegate_(delegate),
34+
context_(nullptr) {
3435
running_.store(false);
3536
}
3637

3738
Executor::~Executor() {
3839
// Note: don't free this->async_ in ctor
3940
}
4041

41-
void Executor::Start(rcl_context_t* context) {
42+
void Executor::Start(rcl_context_t* context, int32_t time_out) {
4243
if (!running_.load()) {
4344
async_ = new uv_async_t();
4445
context_ = context;
46+
time_out_ = time_out;
47+
4548
uv_async_init(uv_default_loop(), async_, DoWork);
4649
async_->data = this;
4750

@@ -96,8 +99,9 @@ void Executor::Run(void* arg) {
9699

97100
try {
98101
rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
99-
rcl_ret_t ret = rcl_wait_set_init(&wait_set, 0, 2, 0, 0, 0, 0,
100-
executor->context_, rcl_get_default_allocator());
102+
rcl_ret_t ret =
103+
rcl_wait_set_init(&wait_set, 0, 2, 0, 0, 0, 0, executor->context_,
104+
rcl_get_default_allocator());
101105
if (ret != RCL_RET_OK) {
102106
throw std::runtime_error(std::string("Init waitset failed: ") +
103107
rcl_get_error_string().str);
@@ -112,44 +116,52 @@ void Executor::Run(void* arg) {
112116
if (handle_manager->is_empty())
113117
continue;
114118

115-
if (rcl_wait_set_resize(
116-
&wait_set,
117-
handle_manager->subscription_count(),
118-
// TODO(minggang): support guard conditions
119-
0u,
120-
handle_manager->timer_count(),
121-
handle_manager->client_count(),
122-
handle_manager->service_count(),
123-
// TODO(minggang): support events.
124-
0u) != RCL_RET_OK) {
125-
std::string error_message = std::string("Failed to resize: ")
126-
+ std::string(rcl_get_error_string().str);
127-
throw std::runtime_error(error_message);
128-
}
119+
if (rcl_wait_set_resize(&wait_set, handle_manager->subscription_count(),
120+
// TODO(minggang): support guard conditions
121+
1u, handle_manager->timer_count(),
122+
handle_manager->client_count(),
123+
handle_manager->service_count(),
124+
// TODO(minggang): support events.
125+
0u) != RCL_RET_OK) {
126+
std::string error_message = std::string("Failed to resize: ") +
127+
std::string(rcl_get_error_string().str);
128+
throw std::runtime_error(error_message);
129+
}
129130

130131
if (!handle_manager->AddHandlesToWaitSet(&wait_set)) {
131132
throw std::runtime_error("Couldn't fill waitset");
132133
}
133134

134-
rcl_ret_t status = rcl_wait(&wait_set, RCL_MS_TO_NS(10));
135+
rcl_wait_set_add_guard_condition(&wait_set, g_sigint_gc, nullptr);
136+
137+
int64_t time_out =
138+
executor->time_out() < 0 ? -1 : RCL_MS_TO_NS(executor->time_out());
139+
140+
rcl_ret_t status = rcl_wait(&wait_set, time_out);
135141
if (status == RCL_RET_WAIT_SET_EMPTY) {
136142
} else if (status != RCL_RET_OK && status != RCL_RET_TIMEOUT) {
137143
throw std::runtime_error(std::string("rcl_wait() failed: ") +
138144
rcl_get_error_string().str);
139145
} else {
146+
if (wait_set.size_of_guard_conditions == 1 &&
147+
wait_set.guard_conditions[0]) {
148+
executor->running_.store(false);
149+
}
140150
if (!uv_is_closing(
141151
reinterpret_cast<uv_handle_t*>(executor->async_))) {
142152
uv_async_send(executor->async_);
143153
}
144154
}
145155

146156
if (rcl_wait_set_clear(&wait_set) != RCL_RET_OK) {
147-
std::string error_message = std::string("Failed to clear wait set: ")
148-
+ std::string(rcl_get_error_string().str);
157+
std::string error_message =
158+
std::string("Failed to clear wait set: ") +
159+
std::string(rcl_get_error_string().str);
149160
throw std::runtime_error(error_message);
150161
}
151162
}
152163
}
164+
153165
if (rcl_wait_set_fini(&wait_set) != RCL_RET_OK) {
154166
throw std::runtime_error(std::string("Failed to destroy guard waitset:") +
155167
rcl_get_error_string().str);

src/executor.hpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,9 @@ class Executor {
3737
Executor(HandleManager* handle_manager, Delegate* delegate);
3838
~Executor();
3939

40-
void Start(rcl_context_t* context);
40+
void Start(rcl_context_t* context, int32_t time_out);
4141
void Stop();
42+
int32_t time_out() { return time_out_; }
4243

4344
static void DoWork(uv_async_t* handle);
4445
static void Run(void* arg);
@@ -50,6 +51,7 @@ class Executor {
5051
HandleManager* handle_manager_;
5152
Delegate* delegate_;
5253
rcl_context_t* context_;
54+
int32_t time_out_;
5355

5456
std::atomic_bool running_;
5557
};

src/handle_manager.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ bool HandleManager::AddHandlesToWaitSet(rcl_wait_set_t* wait_set) {
7070
return false;
7171
}
7272
for (auto& subscription : subscriptions_) {
73-
if (rcl_wait_set_add_subscription(
74-
wait_set, subscription, nullptr) != RCL_RET_OK)
73+
if (rcl_wait_set_add_subscription(wait_set, subscription, nullptr) !=
74+
RCL_RET_OK)
7575
return false;
7676
}
7777
for (auto& client : clients_) {

0 commit comments

Comments
 (0)