Skip to content

Commit d7d79f2

Browse files
committed
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 <jasnell@gmail.com> PR-URL: #33912 Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Denys Otrishko <shishugi@gmail.com>
1 parent 16116f5 commit d7d79f2

File tree

3 files changed

+22
-14
lines changed

3 files changed

+22
-14
lines changed

src/quic/node_quic_socket-inl.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ namespace quic {
1818
std::unique_ptr<QuicPacket> QuicPacket::Create(
1919
const char* diagnostic_label,
2020
size_t len) {
21+
CHECK_LE(len, MAX_PKTLEN);
2122
return std::make_unique<QuicPacket>(diagnostic_label, len);
2223
}
2324

@@ -27,8 +28,8 @@ std::unique_ptr<QuicPacket> QuicPacket::Copy(
2728
}
2829

2930
void QuicPacket::set_length(size_t len) {
30-
CHECK_LE(len, data_.size());
31-
data_.resize(len);
31+
CHECK_LE(len, MAX_PKTLEN);
32+
len_ = len;
3233
}
3334

3435
int QuicEndpoint::Send(

src/quic/node_quic_socket.cc

+6-7
Original file line numberDiff line numberDiff line change
@@ -83,24 +83,23 @@ bool IsShortHeader(
8383
} // namespace
8484

8585
QuicPacket::QuicPacket(const char* diagnostic_label, size_t len) :
86-
data_(len),
86+
data_{0},
87+
len_(len),
8788
diagnostic_label_(diagnostic_label) {
88-
CHECK_LE(len, NGTCP2_MAX_PKT_SIZE);
89+
CHECK_LE(len, MAX_PKTLEN);
8990
}
9091

9192
QuicPacket::QuicPacket(const QuicPacket& other) :
92-
QuicPacket(other.diagnostic_label_, other.data_.size()) {
93-
memcpy(data_.data(), other.data_.data(), other.data_.size());
93+
QuicPacket(other.diagnostic_label_, other.len_) {
94+
memcpy(&data_, &other.data_, other.len_);
9495
}
9596

9697
const char* QuicPacket::diagnostic_label() const {
9798
return diagnostic_label_ != nullptr ?
9899
diagnostic_label_ : "unspecified";
99100
}
100101

101-
void QuicPacket::MemoryInfo(MemoryTracker* tracker) const {
102-
tracker->TrackField("data", data_);
103-
}
102+
void QuicPacket::MemoryInfo(MemoryTracker* tracker) const {}
104103

105104
QuicSocketListener::~QuicSocketListener() {
106105
if (socket_)

src/quic/node_quic_socket.h

+13-5
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,13 @@ class JSQuicSocketListener : public QuicSocketListener {
112112
void OnDestroy() override;
113113
};
114114

115+
// This is just a formality as the QUIC spec normatively
116+
// defines that the ipv4 max pktlen is always going to be
117+
// larger than the ipv6 max pktlen, but in the off chance
118+
// ever changes (which is unlikely) we check here.
119+
constexpr size_t MAX_PKTLEN =
120+
std::max<size_t>(NGTCP2_MAX_PKTLEN_IPV4, NGTCP2_MAX_PKTLEN_IPV6);
121+
115122
// A serialized QuicPacket to be sent by a QuicSocket instance.
116123
class QuicPacket : public MemoryRetainer {
117124
public:
@@ -141,7 +148,7 @@ class QuicPacket : public MemoryRetainer {
141148
// QuicPacket instance will be freed.
142149
static inline std::unique_ptr<QuicPacket> Create(
143150
const char* diagnostic_label = nullptr,
144-
size_t len = NGTCP2_MAX_PKTLEN_IPV4);
151+
size_t len = MAX_PKTLEN);
145152

146153
// Copy the data of the QuicPacket to a new one. Currently,
147154
// this is only used when retransmitting close connection
@@ -151,11 +158,11 @@ class QuicPacket : public MemoryRetainer {
151158

152159
QuicPacket(const char* diagnostic_label, size_t len);
153160
QuicPacket(const QuicPacket& other);
154-
uint8_t* data() { return data_.data(); }
155-
size_t length() const { return data_.size(); }
161+
uint8_t* data() { return data_; }
162+
size_t length() const { return len_; }
156163
uv_buf_t buf() const {
157164
return uv_buf_init(
158-
const_cast<char*>(reinterpret_cast<const char*>(data_.data())),
165+
const_cast<char*>(reinterpret_cast<const char*>(&data_)),
159166
length());
160167
}
161168
inline void set_length(size_t len);
@@ -166,7 +173,8 @@ class QuicPacket : public MemoryRetainer {
166173
SET_SELF_SIZE(QuicPacket);
167174

168175
private:
169-
std::vector<uint8_t> data_;
176+
uint8_t data_[MAX_PKTLEN];
177+
size_t len_ = MAX_PKTLEN;
170178
const char* diagnostic_label_ = nullptr;
171179
};
172180

0 commit comments

Comments
 (0)