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>

Author: jasnell

node.gyp

@@ -354,6 +354,7 @@
       'src/quic/tlscontext.cc',
       'src/quic/transportparams.cc',
       'src/quic/quic.cc',
+      'src/quic/arena.h',
       'src/quic/bindingdata.h',
       'src/quic/cid.h',
       'src/quic/data.h',
@@ -440,6 +441,7 @@
       'test/cctest/test_node_crypto_env.cc',
     ],
     'node_cctest_quic_sources': [
+      'test/cctest/test_quic_arena.cc',
       'test/cctest/test_quic_cid.cc',
       'test/cctest/test_quic_error.cc',
       'test/cctest/test_quic_preferredaddress.cc',

src/async_wrap.h

@@ -63,7 +63,6 @@ namespace node {
   V(QUERYWRAP)                                                                 \
   V(QUIC_ENDPOINT)                                                             \
   V(QUIC_LOGSTREAM)                                                            \
-  V(QUIC_PACKET)                                                               \
   V(QUIC_SESSION)                                                              \
   V(QUIC_STREAM)                                                               \
   V(QUIC_UDP)                                                                  \

src/quic/application.cc

@@ -1,7 +1,6 @@
 #if HAVE_OPENSSL && HAVE_QUIC
 #include "guard.h"
 #ifndef OPENSSL_NO_QUIC
-#include "application.h"
 #include <async_wrap-inl.h>
 #include <debug_utils-inl.h>
 #include <nghttp3/nghttp3.h>
@@ -10,6 +9,7 @@
 #include <node_sockaddr-inl.h>
 #include <uv.h>
 #include <v8.h>
+#include "application.h"
 #include "defs.h"
 #include "endpoint.h"
 #include "http3.h"
@@ -207,12 +207,9 @@ StreamPriority Session::Application::GetStreamPriority(const Stream& stream) {
   return StreamPriority::DEFAULT;
 }
 
-BaseObjectPtr<Packet> Session::Application::CreateStreamDataPacket() {
-  return Packet::Create(env(),
-                        session_->endpoint(),
-                        session_->remote_address(),
-                        session_->max_packet_size(),
-                        "stream data");
+Packet::Ptr Session::Application::CreateStreamDataPacket() {
+  return session_->endpoint().CreatePacket(
+      session_->remote_address(), session_->max_packet_size(), "stream data");
 }
 
 void Session::Application::StreamClose(Stream* stream, QuicError&& error) {
@@ -264,7 +261,7 @@ void Session::Application::SendPendingData() {
   // The number of packets that have been sent in this call to SendPendingData.
   size_t packet_send_count = 0;
 
-  BaseObjectPtr<Packet> packet;
+  Packet::Ptr packet;
   uint8_t* pos = nullptr;
   uint8_t* begin = nullptr;
 
@@ -273,7 +270,7 @@ void Session::Application::SendPendingData() {
       packet = CreateStreamDataPacket();
       if (!packet) [[unlikely]]
         return false;
-      pos = begin = ngtcp2_vec(*packet).base;
+      pos = begin = packet->data();
     }
     DCHECK(packet);
     DCHECK_NOT_NULL(pos);
@@ -299,7 +296,6 @@ void Session::Application::SendPendingData() {
     // The stream_data is the next block of data from the application stream.
     if (GetStreamData(&stream_data) < 0) {
       Debug(session_, "Application failed to get stream data");
-      packet->CancelPacket();
       session_->SetLastError(QuicError::ForNgtcp2Error(NGTCP2_ERR_INTERNAL));
       closed = true;
       return session_->Close(CloseMethod::SILENT);
@@ -367,7 +363,6 @@ void Session::Application::SendPendingData() {
           if (ndatalen >= 0 && !StreamCommit(&stream_data, ndatalen)) {
             Debug(session_,
                   "Failed to commit stream data while writing packets");
-            packet->CancelPacket();
             session_->SetLastError(
                 QuicError::ForNgtcp2Error(NGTCP2_ERR_INTERNAL));
             closed = true;
@@ -380,7 +375,6 @@ void Session::Application::SendPendingData() {
           // ngtcp2 callback failed for some reason. This would be a
           // bug in our code.
           Debug(session_, "Internal failure with ngtcp2 callback");
-          packet->CancelPacket();
           session_->SetLastError(
               QuicError::ForNgtcp2Error(NGTCP2_ERR_INTERNAL));
           closed = true;
@@ -393,12 +387,10 @@ void Session::Application::SendPendingData() {
       Debug(session_,
             "Application encountered error while writing packet: %s",
             ngtcp2_strerror(nwrite));
-      packet->CancelPacket();
       session_->SetLastError(QuicError::ForNgtcp2Error(nwrite));
       closed = true;
       return session_->Close(CloseMethod::SILENT);
     } else if (ndatalen >= 0 && !StreamCommit(&stream_data, ndatalen)) {
-      packet->CancelPacket();
       session_->SetLastError(QuicError::ForNgtcp2Error(NGTCP2_ERR_INTERNAL));
       closed = true;
       return session_->Close(CloseMethod::SILENT);
@@ -416,10 +408,9 @@ void Session::Application::SendPendingData() {
       if (datalen) {
         Debug(session_, "Sending packet with %zu bytes", datalen);
         packet->Truncate(datalen);
-        session_->Send(packet, path);
-      } else {
-        packet->CancelPacket();
+        session_->Send(std::move(packet), path);
       }
+      // If no data, Ptr destructor releases the packet.
 
       return;
     }
@@ -429,15 +420,15 @@ void Session::Application::SendPendingData() {
     size_t datalen = pos - begin;
     Debug(session_, "Sending packet with %zu bytes", datalen);
     packet->Truncate(datalen);
-    session_->Send(packet, path);
+    session_->Send(std::move(packet), path);
 
     // If we have sent the maximum number of packets, we're done.
     if (++packet_send_count == max_packet_count) {
       return;
     }
 
     // Prepare to loop back around to prepare a new packet.
-    packet.reset();
+    // packet is already empty from the std::move above.
     pos = begin = nullptr;
   }
 }

src/quic/application.h

@@ -132,7 +132,7 @@ class Session::Application : public MemoryRetainer {
   }
 
  private:
-  BaseObjectPtr<Packet> CreateStreamDataPacket();
+  Packet::Ptr CreateStreamDataPacket();
 
   // Write the given stream_data into the buffer.
   ssize_t WriteVStream(PathStorage* path,