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 7cda430

Browse filesBrowse files
jasnelladuh95
jasnell
authored and
aduh95
committed
quic: use arena allocation for packets
Previously Packets were ReqWrap objects with a shared free-list. This commit changes to a per-Endpoint arena with no v8 involvement. This is the design I originally had in mind but I initially went with the simpler freelist approach to get something working. There's too much overhead in the reqrap/freelist approach and individual packets do not really need to be observable via async hooks. This design should eliminate the risk of memory fragmentation and eliminate a significant bottleneck in the hot path. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode:Opus 4.6 PR-URL: #62589 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
1 parent d7317f4 commit 7cda430
Copy full SHA for 7cda430

14 files changed

+862-462Lines changed: 862 additions & 462 deletions
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎node.gyp‎

Copy file name to clipboardExpand all lines: node.gyp
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,7 @@
354354
'src/quic/tlscontext.cc',
355355
'src/quic/transportparams.cc',
356356
'src/quic/quic.cc',
357+
'src/quic/arena.h',
357358
'src/quic/bindingdata.h',
358359
'src/quic/cid.h',
359360
'src/quic/data.h',
@@ -440,6 +441,7 @@
440441
'test/cctest/test_node_crypto_env.cc',
441442
],
442443
'node_cctest_quic_sources': [
444+
'test/cctest/test_quic_arena.cc',
443445
'test/cctest/test_quic_cid.cc',
444446
'test/cctest/test_quic_error.cc',
445447
'test/cctest/test_quic_preferredaddress.cc',
Collapse file

‎src/async_wrap.h‎

Copy file name to clipboardExpand all lines: src/async_wrap.h
-1Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ namespace node {
6363
V(QUERYWRAP) \
6464
V(QUIC_ENDPOINT) \
6565
V(QUIC_LOGSTREAM) \
66-
V(QUIC_PACKET) \
6766
V(QUIC_SESSION) \
6867
V(QUIC_STREAM) \
6968
V(QUIC_UDP) \
Collapse file

‎src/quic/application.cc‎

Copy file name to clipboardExpand all lines: src/quic/application.cc
+10-19Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
#if HAVE_OPENSSL && HAVE_QUIC
22
#include "guard.h"
33
#ifndef OPENSSL_NO_QUIC
4-
#include "application.h"
54
#include <async_wrap-inl.h>
65
#include <debug_utils-inl.h>
76
#include <nghttp3/nghttp3.h>
@@ -10,6 +9,7 @@
109
#include <node_sockaddr-inl.h>
1110
#include <uv.h>
1211
#include <v8.h>
12+
#include "application.h"
1313
#include "defs.h"
1414
#include "endpoint.h"
1515
#include "http3.h"
@@ -207,12 +207,9 @@ StreamPriority Session::Application::GetStreamPriority(const Stream& stream) {
207207
return StreamPriority::DEFAULT;
208208
}
209209

210-
BaseObjectPtr<Packet> Session::Application::CreateStreamDataPacket() {
211-
return Packet::Create(env(),
212-
session_->endpoint(),
213-
session_->remote_address(),
214-
session_->max_packet_size(),
215-
"stream data");
210+
Packet::Ptr Session::Application::CreateStreamDataPacket() {
211+
return session_->endpoint().CreatePacket(
212+
session_->remote_address(), session_->max_packet_size(), "stream data");
216213
}
217214

218215
void Session::Application::StreamClose(Stream* stream, QuicError&& error) {
@@ -264,7 +261,7 @@ void Session::Application::SendPendingData() {
264261
// The number of packets that have been sent in this call to SendPendingData.
265262
size_t packet_send_count = 0;
266263

267-
BaseObjectPtr<Packet> packet;
264+
Packet::Ptr packet;
268265
uint8_t* pos = nullptr;
269266
uint8_t* begin = nullptr;
270267

@@ -273,7 +270,7 @@ void Session::Application::SendPendingData() {
273270
packet = CreateStreamDataPacket();
274271
if (!packet) [[unlikely]]
275272
return false;
276-
pos = begin = ngtcp2_vec(*packet).base;
273+
pos = begin = packet->data();
277274
}
278275
DCHECK(packet);
279276
DCHECK_NOT_NULL(pos);
@@ -299,7 +296,6 @@ void Session::Application::SendPendingData() {
299296
// The stream_data is the next block of data from the application stream.
300297
if (GetStreamData(&stream_data) < 0) {
301298
Debug(session_, "Application failed to get stream data");
302-
packet->CancelPacket();
303299
session_->SetLastError(QuicError::ForNgtcp2Error(NGTCP2_ERR_INTERNAL));
304300
closed = true;
305301
return session_->Close(CloseMethod::SILENT);
@@ -367,7 +363,6 @@ void Session::Application::SendPendingData() {
367363
if (ndatalen >= 0 && !StreamCommit(&stream_data, ndatalen)) {
368364
Debug(session_,
369365
"Failed to commit stream data while writing packets");
370-
packet->CancelPacket();
371366
session_->SetLastError(
372367
QuicError::ForNgtcp2Error(NGTCP2_ERR_INTERNAL));
373368
closed = true;
@@ -380,7 +375,6 @@ void Session::Application::SendPendingData() {
380375
// ngtcp2 callback failed for some reason. This would be a
381376
// bug in our code.
382377
Debug(session_, "Internal failure with ngtcp2 callback");
383-
packet->CancelPacket();
384378
session_->SetLastError(
385379
QuicError::ForNgtcp2Error(NGTCP2_ERR_INTERNAL));
386380
closed = true;
@@ -393,12 +387,10 @@ void Session::Application::SendPendingData() {
393387
Debug(session_,
394388
"Application encountered error while writing packet: %s",
395389
ngtcp2_strerror(nwrite));
396-
packet->CancelPacket();
397390
session_->SetLastError(QuicError::ForNgtcp2Error(nwrite));
398391
closed = true;
399392
return session_->Close(CloseMethod::SILENT);
400393
} else if (ndatalen >= 0 && !StreamCommit(&stream_data, ndatalen)) {
401-
packet->CancelPacket();
402394
session_->SetLastError(QuicError::ForNgtcp2Error(NGTCP2_ERR_INTERNAL));
403395
closed = true;
404396
return session_->Close(CloseMethod::SILENT);
@@ -416,10 +408,9 @@ void Session::Application::SendPendingData() {
416408
if (datalen) {
417409
Debug(session_, "Sending packet with %zu bytes", datalen);
418410
packet->Truncate(datalen);
419-
session_->Send(packet, path);
420-
} else {
421-
packet->CancelPacket();
411+
session_->Send(std::move(packet), path);
422412
}
413+
// If no data, Ptr destructor releases the packet.
423414

424415
return;
425416
}
@@ -429,15 +420,15 @@ void Session::Application::SendPendingData() {
429420
size_t datalen = pos - begin;
430421
Debug(session_, "Sending packet with %zu bytes", datalen);
431422
packet->Truncate(datalen);
432-
session_->Send(packet, path);
423+
session_->Send(std::move(packet), path);
433424

434425
// If we have sent the maximum number of packets, we're done.
435426
if (++packet_send_count == max_packet_count) {
436427
return;
437428
}
438429

439430
// Prepare to loop back around to prepare a new packet.
440-
packet.reset();
431+
// packet is already empty from the std::move above.
441432
pos = begin = nullptr;
442433
}
443434
}
Collapse file

‎src/quic/application.h‎

Copy file name to clipboardExpand all lines: src/quic/application.h
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ class Session::Application : public MemoryRetainer {
132132
}
133133

134134
private:
135-
BaseObjectPtr<Packet> CreateStreamDataPacket();
135+
Packet::Ptr CreateStreamDataPacket();
136136

137137
// Write the given stream_data into the buffer.
138138
ssize_t WriteVStream(PathStorage* path,

0 commit comments

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