Skip to content
This repository was archived by the owner on Aug 11, 2020. It is now read-only.

quic: hopefully avoid memory fragmentation issue #388

Closed
wants to merge 1 commit into from
Closed

quic: hopefully avoid memory fragmentation issue #388

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/quic/node_quic_socket-inl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ namespace quic {
std::unique_ptr<QuicPacket> QuicPacket::Create(
const char* diagnostic_label,
size_t len) {
CHECK_LE(len, NGTCP2_MAX_PKTLEN_IPV4);
return std::make_unique<QuicPacket>(diagnostic_label, len);
}

Expand All @@ -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, NGTCP2_MAX_PKTLEN_IPV4);
len_ = len;
}

int QuicEndpoint::Send(
Expand Down
11 changes: 5 additions & 6 deletions src/quic/node_quic_socket.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,24 +84,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);
}

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_)
Expand Down
9 changes: 5 additions & 4 deletions src/quic/node_quic_socket.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,11 +151,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);
Expand All @@ -166,7 +166,8 @@ class QuicPacket : public MemoryRetainer {
SET_SELF_SIZE(QuicPacket);

private:
std::vector<uint8_t> data_;
uint8_t data_[NGTCP2_MAX_PKTLEN_IPV4];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR message talks about using NGTCP2_MAX_PKT_SIZE but new code uses NGTCP2_MAX_PKTLEN_IPV4 is that correct? (note that the check in constructor QuicPacket::QuicPacket(const char* diagnostic_label, size_t len) still checks for NGTCP2_MAX_PKT_SIZE)

Also, NGTCP2_MAX_PKT_SIZE recently changed to NGTCP2_DEFAULT_MAX_UDP_PAYLOAD_SIZE (ngtcp2/ngtcp2@38feb91)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll double check on that. May have mentally mixed those up

size_t len_ = NGTCP2_MAX_PKTLEN_IPV4;
const char* diagnostic_label_ = nullptr;
};

Expand Down