Visitar URL original
quic: use arena allocation for packets · nodejs/node@ea4f19a · GitHub
Skip to content

Commit ea4f19a

Browse 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 21e9239 commit ea4f19a

14 files changed

Lines changed: 862 additions & 462 deletions

‎node.gyp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,7 @@
345345
'src/udp_wrap.h',
346346
'src/util.h',
347347
'src/util-inl.h',
348+
'src/quic/arena.h',
348349
'src/quic/bindingdata.h',
349350
'src/quic/cid.h',
350351
'src/quic/data.h',
@@ -427,6 +428,7 @@
427428
'test/cctest/test_crypto_clienthello.cc',
428429
'test/cctest/test_node_crypto.cc',
429430
'test/cctest/test_node_crypto_env.cc',
431+
'test/cctest/test_quic_arena.cc',
430432
'test/cctest/test_quic_cid.cc',
431433
'test/cctest/test_quic_error.cc',
432434
'test/cctest/test_quic_preferredaddress.cc',

‎src/async_wrap.h‎

Lines 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) \

‎src/quic/application.cc‎

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
#if HAVE_OPENSSL
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
}

‎src/quic/application.h‎

Lines 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)