quic: avoid memory fragmentation issue

Original PR: nodejs/quic#388

Previously, QuicPacket was allocating an std::vector<uint8_t> of
NGTCP2_MAX_PKT_SIZE bytes, then the packet would be serialized into the
buffer, and the std::vector would be resized based on the number of
bytes serialized. I suspect the memory fragmentation that you're seeing
is because of those resize operations not freeing memory in chunks that
are aligned with the allocation. This changes QuicPacket to use a stack
allocation that is always NGTCP2_MAX_PKT_SIZE bytes and the size of the
serialized packet is just recorded without any resizing. When the memory
is freed now, it should be freed in large enough chunks to cover
subsequent allocations.

Signed-off-by: James M Snell <[email protected]>

PR-URL: #33912
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>

src/quic/node_quic_socket-inl.h

@@ -18,6 +18,7 @@ namespace quic {
 std::unique_ptr<QuicPacket> QuicPacket::Create(
     const char* diagnostic_label,
     size_t len) {
+  CHECK_LE(len, MAX_PKTLEN);
   return std::make_unique<QuicPacket>(diagnostic_label, len);
 }
 
@@ -27,8 +28,8 @@ std::unique_ptr<QuicPacket> QuicPacket::Copy(
 }
 
 void QuicPacket::set_length(size_t len) {
-  CHECK_LE(len, data_.size());
-  data_.resize(len);
+  CHECK_LE(len, MAX_PKTLEN);
+  len_ = len;
 }
 
 int QuicEndpoint::Send(

src/quic/node_quic_socket.cc

@@ -83,24 +83,23 @@ bool IsShortHeader(
 }  // namespace
 
 QuicPacket::QuicPacket(const char* diagnostic_label, size_t len) :
-    data_(len),
+    data_{0},
+    len_(len),
     diagnostic_label_(diagnostic_label) {
-  CHECK_LE(len, NGTCP2_MAX_PKT_SIZE);
+  CHECK_LE(len, MAX_PKTLEN);
 }
 
 QuicPacket::QuicPacket(const QuicPacket& other) :
-  QuicPacket(other.diagnostic_label_, other.data_.size()) {
-  memcpy(data_.data(), other.data_.data(), other.data_.size());
+  QuicPacket(other.diagnostic_label_, other.len_) {
+  memcpy(&data_, &other.data_, other.len_);
 }
 
 const char* QuicPacket::diagnostic_label() const {
   return diagnostic_label_ != nullptr ?
       diagnostic_label_ : "unspecified";
 }
 
-void QuicPacket::MemoryInfo(MemoryTracker* tracker) const {
-  tracker->TrackField("data", data_);
-}
+void QuicPacket::MemoryInfo(MemoryTracker* tracker) const {}
 
 QuicSocketListener::~QuicSocketListener() {
   if (socket_)

src/quic/node_quic_socket.h

@@ -112,6 +112,13 @@ class JSQuicSocketListener : public QuicSocketListener {
   void OnDestroy() override;
 };
 
+// This is just a formality as the QUIC spec normatively
+// defines that the ipv4 max pktlen is always going to be
+// larger than the ipv6 max pktlen, but in the off chance
+// ever changes (which is unlikely) we check here.
+constexpr size_t MAX_PKTLEN =
+    std::max<size_t>(NGTCP2_MAX_PKTLEN_IPV4, NGTCP2_MAX_PKTLEN_IPV6);
+
 // A serialized QuicPacket to be sent by a QuicSocket instance.
 class QuicPacket : public MemoryRetainer {
  public:
@@ -141,7 +148,7 @@ class QuicPacket : public MemoryRetainer {
   // QuicPacket instance will be freed.
   static inline std::unique_ptr<QuicPacket> Create(
       const char* diagnostic_label = nullptr,
-      size_t len = NGTCP2_MAX_PKTLEN_IPV4);
+      size_t len = MAX_PKTLEN);
 
   // Copy the data of the QuicPacket to a new one. Currently,
   // this is only used when retransmitting close connection
@@ -151,11 +158,11 @@ class QuicPacket : public MemoryRetainer {
 
   QuicPacket(const char* diagnostic_label, size_t len);
   QuicPacket(const QuicPacket& other);
-  uint8_t* data() { return data_.data(); }
-  size_t length() const { return data_.size(); }
+  uint8_t* data() { return data_; }
+  size_t length() const { return len_; }
   uv_buf_t buf() const {
     return uv_buf_init(
-      const_cast<char*>(reinterpret_cast<const char*>(data_.data())),
+      const_cast<char*>(reinterpret_cast<const char*>(&data_)),
       length());
   }
   inline void set_length(size_t len);
@@ -166,7 +173,8 @@ class QuicPacket : public MemoryRetainer {
   SET_SELF_SIZE(QuicPacket);
 
  private:
-  std::vector<uint8_t> data_;
+  uint8_t data_[MAX_PKTLEN];
+  size_t len_ = MAX_PKTLEN;
   const char* diagnostic_label_ = nullptr;
 };