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 600478a

Browse filesBrowse files
addaleaxBridgeAR
addaleax
authored and
BridgeAR
committed
dgram: use uv_udp_try_send()
This improves dgram performance by avoiding unnecessary async operations. One issue with this commit is that it seems hard to actually create conditions under which the fallback path to the async case is actually taken, for all supported OS, so an internal CLI option is used for testing that path. Another caveat is that the lack of an async operation means that there are slight timing differences (essentially `nextTick()` rather than `setImmediate()` for the send callback). PR-URL: #29832 Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
1 parent b309e20 commit 600478a
Copy full SHA for 600478a

File tree

Expand file treeCollapse file tree

11 files changed

+82
-28
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

11 files changed

+82
-28
lines changed
Open diff view settings
Collapse file

‎benchmark/dgram/array-vs-concat.js‎

Copy file name to clipboardExpand all lines: benchmark/dgram/array-vs-concat.js
+14-6Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,17 +29,25 @@ function main({ dur, len, num, type, chunks }) {
2929

3030
function onsendConcat() {
3131
if (sent++ % num === 0) {
32-
for (var i = 0; i < num; i++) {
33-
socket.send(Buffer.concat(chunk), PORT, '127.0.0.1', onsend);
34-
}
32+
// The setImmediate() is necessary to have event loop progress on OSes
33+
// that only perform synchronous I/O on nonblocking UDP sockets.
34+
setImmediate(() => {
35+
for (var i = 0; i < num; i++) {
36+
socket.send(Buffer.concat(chunk), PORT, '127.0.0.1', onsend);
37+
}
38+
});
3539
}
3640
}
3741

3842
function onsendMulti() {
3943
if (sent++ % num === 0) {
40-
for (var i = 0; i < num; i++) {
41-
socket.send(chunk, PORT, '127.0.0.1', onsend);
42-
}
44+
// The setImmediate() is necessary to have event loop progress on OSes
45+
// that only perform synchronous I/O on nonblocking UDP sockets.
46+
setImmediate(() => {
47+
for (var i = 0; i < num; i++) {
48+
socket.send(chunk, PORT, '127.0.0.1', onsend);
49+
}
50+
});
4351
}
4452
}
4553

Collapse file

‎benchmark/dgram/multi-buffer.js‎

Copy file name to clipboardExpand all lines: benchmark/dgram/multi-buffer.js
+7-3Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,13 @@ function main({ dur, len, num, type, chunks }) {
2727

2828
function onsend() {
2929
if (sent++ % num === 0) {
30-
for (var i = 0; i < num; i++) {
31-
socket.send(chunk, PORT, '127.0.0.1', onsend);
32-
}
30+
// The setImmediate() is necessary to have event loop progress on OSes
31+
// that only perform synchronous I/O on nonblocking UDP sockets.
32+
setImmediate(() => {
33+
for (var i = 0; i < num; i++) {
34+
socket.send(chunk, PORT, '127.0.0.1', onsend);
35+
}
36+
});
3337
}
3438
}
3539

Collapse file

‎benchmark/dgram/offset-length.js‎

Copy file name to clipboardExpand all lines: benchmark/dgram/offset-length.js
+7-3Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,13 @@ function main({ dur, len, num, type }) {
2323

2424
function onsend() {
2525
if (sent++ % num === 0) {
26-
for (var i = 0; i < num; i++) {
27-
socket.send(chunk, 0, chunk.length, PORT, '127.0.0.1', onsend);
28-
}
26+
// The setImmediate() is necessary to have event loop progress on OSes
27+
// that only perform synchronous I/O on nonblocking UDP sockets.
28+
setImmediate(() => {
29+
for (var i = 0; i < num; i++) {
30+
socket.send(chunk, 0, chunk.length, PORT, '127.0.0.1', onsend);
31+
}
32+
});
2933
}
3034
}
3135

Collapse file

‎benchmark/dgram/single-buffer.js‎

Copy file name to clipboardExpand all lines: benchmark/dgram/single-buffer.js
+7-3Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,13 @@ function main({ dur, len, num, type }) {
2323

2424
function onsend() {
2525
if (sent++ % num === 0) {
26-
for (var i = 0; i < num; i++) {
27-
socket.send(chunk, PORT, '127.0.0.1', onsend);
28-
}
26+
// The setImmediate() is necessary to have event loop progress on OSes
27+
// that only perform synchronous I/O on nonblocking UDP sockets.
28+
setImmediate(() => {
29+
for (var i = 0; i < num; i++) {
30+
socket.send(chunk, PORT, '127.0.0.1', onsend);
31+
}
32+
});
2933
}
3034
}
3135

Collapse file

‎lib/dgram.js‎

Copy file name to clipboardExpand all lines: lib/dgram.js
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,14 @@ function doSend(ex, self, ip, list, address, port, callback) {
666666
else
667667
err = state.handle.send(req, list, list.length, !!callback);
668668

669+
if (err >= 1) {
670+
// Synchronous finish. The return code is msg_length + 1 so that we can
671+
// distinguish between synchronous success and asynchronous success.
672+
if (callback)
673+
process.nextTick(callback, null, err - 1);
674+
return;
675+
}
676+
669677
if (err && callback) {
670678
// Don't emit as error, dgram_legacy.js compatibility
671679
const ex = exceptionWithHostPort(err, 'send', address, port);
Collapse file

‎src/node_options.cc‎

Copy file name to clipboardExpand all lines: src/node_options.cc
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,8 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
466466
"write warnings to file instead of stderr",
467467
&EnvironmentOptions::redirect_warnings,
468468
kAllowedInEnvironment);
469+
AddOption("--test-udp-no-try-send", "", // For testing only.
470+
&EnvironmentOptions::test_udp_no_try_send);
469471
AddOption("--throw-deprecation",
470472
"throw an exception on deprecations",
471473
&EnvironmentOptions::throw_deprecation,
Collapse file

‎src/node_options.h‎

Copy file name to clipboardExpand all lines: src/node_options.h
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ class EnvironmentOptions : public Options {
137137
bool heap_prof = false;
138138
#endif // HAVE_INSPECTOR
139139
std::string redirect_warnings;
140+
bool test_udp_no_try_send = false;
140141
bool throw_deprecation = false;
141142
bool trace_deprecation = false;
142143
bool trace_sync_io = false;
Collapse file

‎src/udp_wrap.cc‎

Copy file name to clipboardExpand all lines: src/udp_wrap.cc
+33-11Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -429,11 +429,6 @@ void UDPWrap::DoSend(const FunctionCallbackInfo<Value>& args, int family) {
429429
size_t count = args[2].As<Uint32>()->Value();
430430
const bool have_callback = sendto ? args[5]->IsTrue() : args[3]->IsTrue();
431431

432-
SendWrap* req_wrap;
433-
{
434-
AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(wrap);
435-
req_wrap = new SendWrap(env, req_wrap_obj, have_callback);
436-
}
437432
size_t msg_size = 0;
438433

439434
MaybeStackBuffer<uv_buf_t, 16> bufs(count);
@@ -448,8 +443,6 @@ void UDPWrap::DoSend(const FunctionCallbackInfo<Value>& args, int family) {
448443
msg_size += length;
449444
}
450445

451-
req_wrap->msg_size = msg_size;
452-
453446
int err = 0;
454447
struct sockaddr_storage addr_storage;
455448
sockaddr* addr = nullptr;
@@ -462,18 +455,47 @@ void UDPWrap::DoSend(const FunctionCallbackInfo<Value>& args, int family) {
462455
}
463456
}
464457

458+
uv_buf_t* bufs_ptr = *bufs;
459+
if (err == 0 && !UNLIKELY(env->options()->test_udp_no_try_send)) {
460+
err = uv_udp_try_send(&wrap->handle_, bufs_ptr, count, addr);
461+
if (err == UV_ENOSYS || err == UV_EAGAIN) {
462+
err = 0;
463+
} else if (err >= 0) {
464+
size_t sent = err;
465+
while (count > 0 && bufs_ptr->len <= sent) {
466+
sent -= bufs_ptr->len;
467+
bufs_ptr++;
468+
count--;
469+
}
470+
if (count > 0) {
471+
CHECK_LT(sent, bufs_ptr->len);
472+
bufs_ptr->base += sent;
473+
bufs_ptr->len -= sent;
474+
} else {
475+
CHECK_EQ(static_cast<size_t>(err), msg_size);
476+
// + 1 so that the JS side can distinguish 0-length async sends from
477+
// 0-length sync sends.
478+
args.GetReturnValue().Set(static_cast<uint32_t>(msg_size) + 1);
479+
return;
480+
}
481+
}
482+
}
483+
465484
if (err == 0) {
485+
AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(wrap);
486+
SendWrap* req_wrap = new SendWrap(env, req_wrap_obj, have_callback);
487+
req_wrap->msg_size = msg_size;
488+
466489
err = req_wrap->Dispatch(uv_udp_send,
467490
&wrap->handle_,
468-
*bufs,
491+
bufs_ptr,
469492
count,
470493
addr,
471494
OnSend);
495+
if (err)
496+
delete req_wrap;
472497
}
473498

474-
if (err)
475-
delete req_wrap;
476-
477499
args.GetReturnValue().Set(err);
478500
}
479501

Collapse file

‎test/async-hooks/test-udpsendwrap.js‎

Copy file name to clipboardExpand all lines: test/async-hooks/test-udpsendwrap.js
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// Flags: --test-udp-no-try-send
12
'use strict';
23

34
const common = require('../common');
Collapse file

‎test/parallel/test-dgram-send-callback-recursive.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-dgram-send-callback-recursive.js
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ function onsend() {
2222
client.on('listening', function() {
2323
port = this.address().port;
2424

25-
setImmediate(function() {
25+
process.nextTick(() => {
2626
async = true;
2727
});
2828

0 commit comments

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