Skip to content

Commit c96d8a9

Browse files
jasnelladuh95
jasnell
authored and
aduh95
committed
quic: improve stream header collection performance
Signed-off-by: James M Snell <[email protected]> Assisted-by: Opencode:Opus 4.6 PR-URL: #63267 Reviewed-By: Matteo Collina <[email protected]>
1 parent 409460f commit c96d8a9

3 files changed

Lines changed: 48 additions & 40 deletions

File tree

‎src/quic/http3.cc‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -333,9 +333,7 @@ class Http3ApplicationImpl final : public Session::Application {
333333
// We cannot add the header if we've either reached
334334
// * the max number of header pairs or
335335
// * the max number of header bytes (name + value combined)
336-
// current_count is the number of entries in the headers vector
337-
// (each pair = name entry + value entry = 2 entries).
338-
return (current_count / 2 < options_.max_header_pairs) &&
336+
return (current_count < options_.max_header_pairs) &&
339337
(current_headers_length + this_header_length) <=
340338
options_.max_header_length;
341339
}
@@ -825,12 +823,12 @@ class Http3ApplicationImpl final : public Session::Application {
825823
stream->BeginHeaders(HeadersKind::INITIAL);
826824
}
827825

828-
void OnReceiveHeader(stream_id id, Http3Header&& header) {
826+
void OnReceiveHeader(stream_id id, std::unique_ptr<Http3Header> header) {
829827
auto stream = session().FindStream(id);
830828

831829
if (!stream) [[unlikely]]
832830
return;
833-
if (header.name() == ":status" && header.value()[0] == '1') {
831+
if (header->name() == ":status" && header->value()[0] == '1') {
834832
Debug(&session(),
835833
"HTTP/3 application switching to hints headers for stream %" PRIi64,
836834
stream->id());
@@ -839,8 +837,8 @@ class Http3ApplicationImpl final : public Session::Application {
839837
IF_QUIC_DEBUG(env()) {
840838
Debug(&session(),
841839
"Received header \"%s: %s\"",
842-
header.name(),
843-
header.value());
840+
header->name(),
841+
header->value());
844842
}
845843
stream->AddHeader(std::move(header));
846844
}
@@ -874,15 +872,15 @@ class Http3ApplicationImpl final : public Session::Application {
874872
stream->BeginHeaders(HeadersKind::TRAILING);
875873
}
876874

877-
void OnReceiveTrailer(stream_id id, Http3Header&& header) {
875+
void OnReceiveTrailer(stream_id id, std::unique_ptr<Http3Header> header) {
878876
auto stream = session().FindStream(id);
879877
if (!stream) [[unlikely]]
880878
return;
881879
IF_QUIC_DEBUG(env()) {
882880
Debug(&session(),
883881
"Received header \"%s: %s\"",
884-
header.name(),
885-
header.value());
882+
header->name(),
883+
header->value());
886884
}
887885
stream->AddHeader(std::move(header));
888886
}
@@ -1239,7 +1237,9 @@ class Http3ApplicationImpl final : public Session::Application {
12391237
return NGHTTP3_ERR_CALLBACK_FAILURE;
12401238
}
12411239
if (Http3Header::IsZeroLength(token, name, value)) return NGTCP2_SUCCESS;
1242-
app.OnReceiveHeader(id, Http3Header(app.env(), token, name, value, flags));
1240+
app.OnReceiveHeader(
1241+
id,
1242+
std::make_unique<Http3Header>(app.env(), token, name, value, flags));
12431243
return NGTCP2_SUCCESS;
12441244
}
12451245

@@ -1281,7 +1281,9 @@ class Http3ApplicationImpl final : public Session::Application {
12811281
return NGHTTP3_ERR_CALLBACK_FAILURE;
12821282
}
12831283
if (Http3Header::IsZeroLength(token, name, value)) return NGTCP2_SUCCESS;
1284-
app.OnReceiveTrailer(id, Http3Header(app.env(), token, name, value, flags));
1284+
app.OnReceiveTrailer(
1285+
id,
1286+
std::make_unique<Http3Header>(app.env(), token, name, value, flags));
12851287
return NGTCP2_SUCCESS;
12861288
}
12871289

‎src/quic/streams.cc‎

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ using v8::HandleScope;
3131
using v8::Integer;
3232
using v8::Just;
3333
using v8::Local;
34+
using v8::LocalVector;
3435
using v8::Maybe;
3536
using v8::Nothing;
3637
using v8::Object;
@@ -1126,8 +1127,7 @@ Stream::Stream(BaseObjectWeakPtr<Session> session,
11261127
std::shared_ptr<DataQueue> source)
11271128
: AsyncWrap(session->env(), object, PROVIDER_QUIC_STREAM),
11281129
session_(std::move(session)),
1129-
inbound_(DataQueue::Create()),
1130-
headers_(env()->isolate()) {
1130+
inbound_(DataQueue::Create()) {
11311131
auto& binding = BindingData::Get(env());
11321132
stats_slot_ = GetStreamStatsArena(binding).Allocate(env()->isolate());
11331133
state_slot_ = GetStreamStateArena(binding).Allocate(env()->isolate());
@@ -1185,8 +1185,7 @@ Stream::Stream(BaseObjectWeakPtr<Session> session,
11851185
session_(std::move(session)),
11861186
inbound_(DataQueue::Create()),
11871187
maybe_pending_stream_(
1188-
std::make_unique<PendingStream>(direction, this, session_)),
1189-
headers_(env()->isolate()) {
1188+
std::make_unique<PendingStream>(direction, this, session_)) {
11901189
auto& binding = BindingData::Get(env());
11911190
stats_slot_ = GetStreamStatsArena(binding).Allocate(env()->isolate());
11921191
state_slot_ = GetStreamStateArena(binding).Allocate(env()->isolate());
@@ -1567,27 +1566,16 @@ void Stream::set_headers_kind(HeadersKind kind) {
15671566
headers_kind_ = kind;
15681567
}
15691568

1570-
bool Stream::AddHeader(const Header& header) {
1571-
size_t len = header.length();
1569+
bool Stream::AddHeader(std::unique_ptr<Header> header) {
1570+
size_t len = header->length();
15721571
if (!session_->application().CanAddHeader(
15731572
headers_.size(), headers_length_, len)) {
15741573
return false;
15751574
}
15761575

15771576
headers_length_ += len;
1578-
1579-
auto& state = BindingData::Get(env());
1580-
1581-
const auto push = [&](auto raw) {
1582-
Local<Value> value;
1583-
if (!raw.ToLocal(&value)) [[unlikely]] {
1584-
return false;
1585-
}
1586-
headers_.push_back(value);
1587-
return true;
1588-
};
1589-
1590-
return push(header.GetName(&state)) && push(header.GetValue(&state));
1577+
headers_.push_back(std::move(header));
1578+
return true;
15911579
}
15921580

15931581
void Stream::Acknowledge(size_t datalen) {
@@ -1899,19 +1887,35 @@ void Stream::EmitHeaders() {
18991887
// state()->wants_headers will be set from the javascript side if the
19001888
// stream object has a handler for the headers event.
19011889
if (!env()->can_call_into_js() || !state()->wants_headers) {
1890+
headers_.clear();
19021891
return;
19031892
}
19041893
CallbackScope<Stream> cb_scope(this);
19051894

1895+
auto& binding = BindingData::Get(env());
1896+
size_t count = headers_.size() * 2;
1897+
LocalVector<Value> values(env()->isolate(), count);
1898+
1899+
for (size_t i = 0; i < headers_.size(); i++) {
1900+
Local<Value> name;
1901+
Local<Value> value;
1902+
if (!headers_[i]->GetName(&binding).ToLocal(&name) ||
1903+
!headers_[i]->GetValue(&binding).ToLocal(&value)) [[unlikely]] {
1904+
headers_.clear();
1905+
return;
1906+
}
1907+
values[i * 2] = name;
1908+
values[i * 2 + 1] = value;
1909+
}
1910+
1911+
headers_.clear();
1912+
19061913
Local<Value> argv[] = {
1907-
Array::New(env()->isolate(), headers_.data(), headers_.size()),
1914+
Array::New(env()->isolate(), values.data(), count),
19081915
Integer::NewFromUnsigned(env()->isolate(),
19091916
static_cast<uint32_t>(headers_kind_))};
19101917

1911-
headers_.clear();
1912-
1913-
MakeCallback(
1914-
BindingData::Get(env()).stream_headers_callback(), arraysize(argv), argv);
1918+
MakeCallback(binding.stream_headers_callback(), arraysize(argv), argv);
19151919
}
19161920

19171921
void Stream::EmitReset(const QuicError& error) {

‎src/quic/streams.h‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ class Stream final : public AsyncWrap,
344344
// Returns false if the header cannot be added. This will typically happen
345345
// if the application does not support headers, a maximum number of headers
346346
// have already been added, or the maximum total header length is reached.
347-
bool AddHeader(const Header& header);
347+
bool AddHeader(std::unique_ptr<Header> header);
348348

349349
// TODO(@jasnell): Implement MemoryInfo to track outbound_, inbound_,
350350
// reader_, headers_, and pending_headers_queue_.
@@ -455,9 +455,11 @@ class Stream final : public AsyncWrap,
455455
const StoredPriority& stored_priority() const { return priority_; }
456456

457457
// The headers_ field holds a block of headers that have been received and
458-
// are being buffered for delivery to the JavaScript side.
459-
// TODO(@jasnell): Use v8::Global instead of v8::Local here.
460-
v8::LocalVector<v8::Value> headers_;
458+
// are being buffered for delivery to the JavaScript side. Headers are
459+
// stored as C++ objects during collection (AddHeader) and converted to
460+
// V8 strings only when emitted (EmitHeaders), avoiding StrongRootAllocator
461+
// mutex contention on the per-header hot path.
462+
std::vector<std::unique_ptr<Header>> headers_;
461463

462464
// The headers_kind_ field indicates the kind of headers that are being
463465
// buffered.

0 commit comments

Comments
 (0)