Skip to content

feat: skip terminal-stream scan when no streams have completed#3595

Merged
larseggert merged 14 commits into
mozilla:mainfrom
larseggert:feat-has_terminal
May 28, 2026
Merged

feat: skip terminal-stream scan when no streams have completed#3595
larseggert merged 14 commits into
mozilla:mainfrom
larseggert:feat-has_terminal

Conversation

@larseggert
Copy link
Copy Markdown
Collaborator

cleanup_closed_streams — called on every timer tick and every input batch — unconditionally scanned all send and recv streams for terminal states. With many concurrent streams this is O(n) work per packet even when nothing has finished.

Add a has_terminal flag to SendStreams and RecvStreams, set precisely when a stream transitions to a terminal state. This allows remove_terminal and clear_terminal to return early when the flag is unset, reducing the common case from O(n) to O(1).

`cleanup_closed_streams` — called on every timer tick and every input
batch — unconditionally scanned all send and recv streams for terminal
states. With many concurrent streams this is O(n) work per packet even
when nothing has finished.

Add a `has_terminal` flag to `SendStreams` and `RecvStreams`, set
precisely when a stream transitions to a terminal state. This allows
`remove_terminal` and `clear_terminal` to return early when the flag is
unset, reducing the common case from O(n) to O(1).
Copilot AI review requested due to automatic review settings May 8, 2026 13:36
@codecov
Copy link
Copy Markdown

codecov Bot commented May 8, 2026

Codecov Report

❌ Patch coverage is 91.91919% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.06%. Comparing base (1b8abbb) to head (4e468a9).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3595      +/-   ##
==========================================
- Coverage   95.18%   95.06%   -0.13%     
==========================================
  Files         111      116       +5     
  Lines       38094    38490     +396     
  Branches    38094    38490     +396     
==========================================
+ Hits        36261    36589     +328     
- Misses       1143     1202      +59     
- Partials      690      699       +9     
Flag Coverage Δ
freebsd 94.21% <91.91%> (-0.13%) ⬇️
linux 95.21% <91.91%> (+0.03%) ⬆️
macos 95.13% <91.91%> (+0.03%) ⬆️
windows 95.19% <91.91%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
neqo-common 98.61% <ø> (ø)
neqo-http3 93.92% <ø> (ø)
neqo-qpack 95.62% <ø> (ø)
neqo-transport 95.90% <92.00%> (+0.04%) ⬆️
neqo-udp 84.69% <ø> (-0.21%) ⬇️
mtu 86.61% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Improve QUIC stream cleanup performance by avoiding O(n) scans of send/recv stream maps when no streams have reached terminal states.

Changes:

  • Add has_terminal fast-path flags to SendStreams and RecvStreams to skip terminal-state scans when unnecessary.
  • Update send-stream terminal cleanup to return whether any streams were removed, enabling dependent recv-side cleanup.
  • Plumb recv-side terminal notifications from Connection::stream_recv() and RESET_STREAM handling.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
neqo-transport/src/streams.rs Uses SendStreams::remove_terminal() -> bool and adds recv-terminal notification hook used by the connection API.
neqo-transport/src/send_stream.rs Adds has_terminal tracking and early-return behavior to make terminal send-stream cleanup O(1) in the common case.
neqo-transport/src/recv_stream.rs Adds has_terminal tracking and early-return behavior to make terminal recv-stream cleanup O(1) in the common case.
neqo-transport/src/connection/mod.rs Marks recv-side terminal transitions when stream_recv() consumes the FIN.
Comments suppressed due to low confidence (1)

neqo-transport/src/streams.rs:337

  • cleanup_closed_streams() now depends on RecvStreams::has_terminal to decide whether clear_terminal() runs. However, a STOP_SENDING ACK can transition a RecvStream to a terminal state (stop_sending_acked() can set ResetRecvd when final_size_reached), and the current STOP_SENDING ack path doesn’t appear to mark recv.note_terminal(). Without flagging that transition, clear_terminal() may never run and terminal uni recv streams may be retained indefinitely. Consider marking recv.note_terminal() when STOP_SENDING is acked (or when stop_sending_acked() results in a terminal state).
        let (removed_bidi, removed_uni) = self.recv.clear_terminal(&self.send, self.role);

        // Send max_streams updates if we removed remote-initiated recv streams.
        // The updates will be send if any streams has been removed.
        self.remote_stream_limits[StreamType::BiDi].add_retired(removed_bidi);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread neqo-transport/src/recv_stream.rs Outdated
Comment thread neqo-transport/src/connection/mod.rs Outdated
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 8, 2026

Merging this PR will not alter performance

✅ 24 untouched benchmarks
⏩ 27 skipped benchmarks1


Comparing larseggert:feat-has_terminal (4e468a9) with main (3707580)

Open in CodSpeed

Footnotes

  1. 27 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Comment thread neqo-transport/src/connection/mod.rs Outdated
Comment thread neqo-transport/src/streams.rs
Comment thread neqo-transport/src/send_stream.rs Outdated
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Clean, well-targeted optimization. The has_terminal flag is a good approach to turn the common-case O(n) scan into O(1). The send-stream side is correctly covered — both acked() and reset_acked() set the flag, and those are the only two paths to a terminal SendStream.

However, two recv-stream terminal-transition paths are not wired up to note_terminal():

  1. Connection::stream_stop_sending() — can transition DataRecvdDataRead (terminal) without setting the flag. Resource leak for uni recv streams.
  2. Streams::acked() / StopSending arm — calls stop_sending_acked(), which can transition AbortReadingResetRecvd (terminal) without setting the flag. Same leak scenario.

For bidi streams both are mitigated by the send.remove_terminal()recv.note_terminal() safety net in cleanup_closed_streams, but uni recv streams have no such fallback.

The fix for both is straightforward: add note_terminal() / note_recv_terminal() calls after the respective operations. The flag is idempotent so spurious sets are cheap. See inline comments for suggested patches.

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Clean, well-scoped optimization. The has_terminal flag correctly turns the common-case O(n) terminal-stream scan into O(1) for both send and recv paths.

Terminal transition coverage — I audited every path that can move a RecvStream or SendStream into a terminal state (DataRead, ResetRecvd, DataRecvd). All are now correctly wired to set the flag:

  • Send side: mark_as_acked (→DataRecvd) and reset_acked (→ResetRecvd) both use the precise has_terminal |= is_terminal() pattern.
  • Recv side: read (fin), stop_sending (DataRecvdDataRead), reset (→ResetRecvd), and stop_sending_acked (AbortReadingResetRecvd) all call note_terminal. The recv-side calls are conservative (fire even when the transition may not reach a terminal state), but the flag is idempotent so the cost is at most one extra scan.

The coupling between remove_terminal returning bool and recv.note_terminal() in cleanup_closed_streams is the key design choice — it ensures retained terminal bidi recv streams (whose send counterpart is still alive) get re-flagged when the send side is eventually cleaned up. This is correct.

The two issues flagged by the prior review (stream_stop_sending and StopSending ack path) are both addressed in 946689f.

Two minor suggestions inline (both non-blocking).

Comment thread neqo-transport/src/send_stream.rs Outdated
Comment thread neqo-transport/src/recv_stream.rs Outdated
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Clean, well-scoped optimization. I audited every path that can move a RecvStream or SendStream into a terminal state and confirmed full coverage at commit 267509b:

Send sideacked() and reset_acked() use the precise has_terminal |= ss.is_terminal() pattern; these are the only two paths to a terminal SendStream.

Recv side — all four terminal-transition paths are wired:

  • read() returning FIN → flagged in Connection::stream_recv() (precise: only when fin=true)
  • stop_sending() on DataRecvd → flagged in Connection::stream_stop_sending() (conservative)
  • reset() → flagged in Streams::input_frame() (conservative)
  • stop_sending_acked() with final_size_reached → flagged in Streams::acked() (conservative)

The conservative recv-side calls (unconditional note_terminal()) are correct — the flag is idempotent and the worst case is one extra scan. The coupling between remove_terminal() -> bool and recv.note_terminal() in cleanup_closed_streams correctly handles retained bidi recv streams whose send counterpart is still alive.

The issues from the first review iteration (stream_stop_sending and StopSending ack path) are both addressed. The #[must_use] and retained-bidi-comment suggestions are also applied.

One minor inline suggestion on a slightly imprecise comment.

Comment thread neqo-transport/src/streams.rs Outdated
Comment thread neqo-transport/src/streams.rs Outdated
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Clean, well-scoped optimization. The has_terminal flag correctly turns the common-case O(n) terminal-stream scan in cleanup_closed_streams (called on every timer tick and input batch) into O(1) when nothing has finished.

I audited every path that can move a RecvStream or SendStream into a terminal state at f0d9085:

Send sideacked() and reset_acked() use the precise has_terminal |= ss.is_terminal() pattern; these are the only two paths to a terminal SendStream.

Recv side — all four terminal-transition paths are wired:

  • read() returning FIN → flagged in Connection::stream_recv (precise: only when fin=true)
  • stop_sending() on DataRecvd → flagged in Connection::stream_stop_sending (conservative)
  • reset() → flagged in Streams::input_frame (conservative)
  • stop_sending_acked() with final_size_reached → flagged in Streams::acked (conservative)

The conservative recv-side calls are correct — the flag is idempotent and the worst case is one extra scan. RecvStream::read() has no other non-test callers besides Connection::stream_recv, so no bypass path exists.

The coupling between remove_terminal() -> bool and recv.note_terminal() in cleanup_closed_streams correctly handles retained bidi recv streams whose send counterpart is still alive (re-flagged when the send side is eventually cleaned up). #[must_use] on remove_terminal is a good defensive measure for future callers.

All issues from prior review iterations (missing stream_stop_sending / StopSending ack paths, #[must_use], rename to stream_ended, comment wording) are addressed.

@larseggert larseggert requested a review from martinthomson May 11, 2026 06:17
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Clean, well-targeted optimization. The has_terminal flag correctly short-circuits the O(n) terminal-stream scans in cleanup_closed_streams (called on every timer tick and input batch) to O(1) in the common case.

I audited every path that can transition a RecvStream or SendStream into a terminal state (DataRead, ResetRecvd for recv; the corresponding states for send) against the current diff at 24ef0fb:

  • Send side: acked() and reset_acked() use the precise has_terminal |= ss.is_terminal() pattern — flag is only set when the stream actually became terminal.
  • Recv side: All four terminal-transition paths (read returning FIN, stop_sending on DataRecvd, reset, stop_sending_acked with final_size_reached) are wired to note_terminal(). The recv-side calls are conservative (fire even when the transition may not reach a terminal state), but the flag is idempotent so the worst case is one extra scan.

The key design coupling — remove_terminal() -> bool gating recv.note_terminal() in cleanup_closed_streams — correctly handles retained terminal bidi recv streams whose send counterpart is still alive, ensuring they get re-flagged when the send side is eventually cleaned up.

All issues from prior reviews (missing stream_stop_sending / StopSending ack paths, #[must_use], rename to stream_ended, comment precision) are addressed. No remaining concerns.

Copy link
Copy Markdown
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

RecvStream::read is a pub API and the "must call stream_ended() on fin" rule lives only in a comment. A future caller might forget. Can we do better? Is it worth the introduced complexity?

@larseggert
Copy link
Copy Markdown
Collaborator Author

@mxinden would you take another look?

@larseggert larseggert requested a review from mxinden May 20, 2026 04:28
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Clean, well-scoped optimization. The latest iteration (wrapping state-transitioning operations in RecvStreams methods that internally track has_ended) is a significant design improvement over the earlier note_terminal() approach — it eliminates the class of bugs where callers forget to flag terminal transitions.

I audited all four recv-stream terminal-transition paths against commit 6525ca1:

  • read() returning FINRecvStreams::read flags precisely on fin=true (only when DataRecvdDataRead)
  • stop_sending() on DataRecvdRecvStreams::stop_sending captures the bool return
  • reset()RecvStreams::reset captures the Res<bool> return
  • stop_sending_acked() with final_size_reachedRecvStreams::stop_sending_acked captures the bool return

Send-side coverage is also complete: acked() and reset_acked() use the precise has_ended |= ss.is_ended() pattern.

The remove_ended() → bool coupling in cleanup_closed_streams correctly handles retained bidi recv streams whose send counterpart is still alive — they get re-flagged when the send side is eventually cleaned up. The #[must_use] on remove_ended guards this invariant for future callers.

Benchmarks confirm measurable wins (up to ~10% on 1000-stream workloads). Tests cover both the stop_sending on DataRecvd and stop_sending_acked with final_size_reached edge cases that were the original gap. All prior review issues are addressed.

Comment thread neqo-transport/src/streams.rs
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Good optimization. Wrapping the state-transitioning RecvStream methods (read, stop_sending, reset, stop_sending_acked) inside RecvStreams methods that internally track has_ended is a clean design that eliminates the caller-must-remember-to-flag bug class from earlier iterations.

I audited all terminal-transition paths at e68f8d8 — send-side (acked, reset_acked) and recv-side (all four paths) are fully covered. The remove_ended() -> bool coupling in cleanup_closed_streams correctly re-flags retained bidi recv streams when their send counterpart is eventually cleaned up.

The two new tests target exactly the edge cases (stop_sending on DataRecvd, stop_sending_acked with final_size_reached) that were the original gap — good coverage.

Two minor inline suggestions for #[must_use] on the bool-returning RecvStream methods, as defense in depth against future direct callers bypassing the RecvStreams wrappers.

Comment thread neqo-transport/src/recv_stream.rs
Comment thread neqo-transport/src/recv_stream.rs
larseggert and others added 2 commits May 27, 2026 03:14
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Signed-off-by: Lars Eggert <lars@eggert.org>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Signed-off-by: Lars Eggert <lars@eggert.org>
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Clean, well-evolved optimization. Wrapping the state-transitioning RecvStream methods (read, stop_sending, reset, stop_sending_acked) inside RecvStreams methods that internally track has_ended is a solid design — it eliminates the caller-must-remember-to-flag bug class that affected earlier iterations and addresses @mxinden's concern about the fragility of the pub API contract.

I independently audited all terminal-transition paths at 186e4bd:

  • Send side: acked() and reset_acked() use the precise has_ended |= ss.is_ended() pattern — only two paths to a terminal SendStream, both covered.
  • Recv side: All four paths (read FIN, stop_sending on DataRecvd, reset, stop_sending_acked with final_size_reached) are captured by the RecvStreams wrappers. The conservative |= on has_ended means the worst case is one extra scan — correct trade-off.

The remove_ended() → bool coupling in cleanup_closed_streams correctly handles retained bidi recv streams whose send counterpart is still alive. #[must_use] on remove_ended, stop_sending, and stop_sending_acked guards the key invariants for future callers.

The two new tests target exactly the edge cases (stop_sending on DataRecvd, stop_sending_acked with final_size_reached) that were the original gap — good coverage.

Two minor inline suggestions, neither blocking.

Comment thread neqo-transport/src/recv_stream.rs Outdated
Comment thread neqo-transport/src/recv_stream.rs
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Clean, well-scoped optimization. The final design — wrapping state-transitioning RecvStream methods inside RecvStreams methods that internally track has_ended — is a significant improvement over the earlier note_terminal() approach. It eliminates the class of bugs where callers forget to flag terminal transitions, which was the main concern across prior review iterations.

I independently audited all terminal-transition paths at the latest commit:

  • Send side: acked() and reset_acked() use the precise has_ended |= ss.is_ended() pattern — the only two paths to a terminal SendStream.
  • Recv side: All four paths are captured by RecvStreams wrappers:
    • read() returning FIN → precise (only when DataRecvdDataRead, i.e., buffered() == 0)
    • stop_sending() on DataRecvd → precise (true only for DataRecvdDataRead)
    • reset() → precise (true only for Recv/SizeKnown/AbortReading/WaitForResetResetRecvd)
    • stop_sending_acked() with final_size_reached → precise (true only for AbortReadingResetRecvd)

The remove_ended() → bool coupling in cleanup_closed_streams correctly handles retained bidi recv streams whose send counterpart is still alive — they get re-flagged when the send side is eventually cleaned up. #[must_use] on remove_ended, stop_sending, and stop_sending_acked guards the key invariants. set_ended is correctly scoped to pub(crate).

No remaining callers of get_recv_stream_mut bypass the wrappers for state-transitioning operations (only stats() and set_stream_max_data() use the direct access path).

The two new tests precisely target the edge cases (stop_sending on DataRecvd, stop_sending_acked with final_size_reached) that motivated the wrapper redesign. All prior review issues are addressed.

Comment thread neqo-transport/src/recv_stream.rs
Comment thread neqo-transport/src/recv_stream.rs
Comment thread neqo-transport/src/send_stream.rs Outdated
Comment thread neqo-transport/src/streams.rs
@larseggert larseggert enabled auto-merge May 28, 2026 06:08
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Clean, well-evolved optimization. The final design — wrapping RecvStream state-transitioning methods inside RecvStreams methods that internally track has_ended — eliminates the caller-must-remember-to-flag bug class from earlier iterations, directly addressing @mxinden's concern about the fragility of the pub API contract.

I independently audited all terminal-transition paths at 6e5171b:

  • Send side: acked() and reset_acked() use the precise has_ended |= ss.is_ended() pattern — the only two paths to a terminal SendStream.
  • Recv side: All four paths are captured by RecvStreams wrappers — read (FIN), stop_sending (DataRecvdDataRead), reset (→ ResetRecvd), stop_sending_acked (final_size_reachedResetRecvd). The remaining get_recv_stream_mut callers (recv_stream_stats, set_stream_max_data) are non-state-transitioning, so no bypass path exists.

The remove_ended() → bool coupling in cleanup_closed_streams correctly handles retained bidi recv streams. The extract_if refactor (per @martinthomson's suggestion) is a nice improvement over retain. All prior review issues are addressed; one minor doc-consistency suggestion inline.

Comment thread neqo-transport/src/recv_stream.rs
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Signed-off-by: Lars Eggert <lars@eggert.org>
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Well-designed optimization that cleanly turns the common-case O(n) terminal-stream scan in cleanup_closed_streams into O(1).

The final design — wrapping RecvStream state-transitioning methods (read, stop_sending, reset, stop_sending_acked) inside RecvStreams methods that internally track has_ended — is a significant improvement over the earlier note_terminal() approach and addresses the fragility concern around callers forgetting to flag transitions.

I independently audited all terminal-transition paths at 4e468a9:

  • Send side: acked() and reset_acked() use the precise has_ended |= ss.is_ended() pattern — the only two paths to a terminal SendStream.
  • Recv side: All four paths are captured by RecvStreams wrappers:
    • read() returning FIN — precise (only when DataRecvdDataRead, i.e., buffered() == 0)
    • stop_sending() on DataRecvd — precise (true only for DataRecvdDataRead)
    • reset() — precise (true only when transitioning to ResetRecvd)
    • stop_sending_acked() with final_size_reached — precise (true only for AbortReadingResetRecvd)

The remove_ended() → bool coupling in cleanup_closed_streams correctly handles retained bidi recv streams whose send counterpart is still alive — they get re-flagged when the send side is eventually cleaned up. #[must_use] on remove_ended, stop_sending, and stop_sending_acked guards these invariants. The remaining direct callers of get_recv_stream_mut (recv_stream_stats, set_stream_max_data) are non-state-transitioning, so no bypass path exists.

The two new tests precisely target the edge cases (stop_sending on DataRecvd, stop_sending_acked with final_size_reached) that motivated the wrapper redesign. The extract_if refactor for SendStreams::remove_ended is a nice cleanup. All prior review issues are addressed.

@larseggert larseggert added this pull request to the merge queue May 28, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Failed Interop Tests

QUIC Interop Runner, client vs. server, differences relative to main at 4ebc148.

neqo-pr as clientneqo-pr as server
neqo-pr vs. go-x-net: BP BA
neqo-pr vs. haproxy: 3 BP BA
neqo-pr vs. kwik: BP BA
neqo-pr vs. lsquic: L1 C1
neqo-pr vs. msquic: A L1 C1
neqo-pr vs. mvfst: A BP BA
neqo-pr vs. neqo: A
neqo-pr vs. nginx: BP BA
neqo-pr vs. ngtcp2: CM
neqo-pr vs. picoquic: A 🚀BP
neqo-pr vs. quic-go: A ⚠️C1
neqo-pr vs. quic-zig: B
neqo-pr vs. quiche: BP BA
neqo-pr vs. s2n-quic: CM
neqo-pr vs. tquic: S BP BA
neqo-pr vs. xquic: A L1 ⚠️C1
aioquic vs. neqo-pr: CM
go-x-net vs. neqo-pr: CM
kwik vs. neqo-pr: BP BA CM
msquic vs. neqo-pr: CM
mvfst vs. neqo-pr: Z A L1 C1 CM
neqo vs. neqo-pr: A
openssl vs. neqo-pr: LR M A CM
quic-go vs. neqo-pr: CM
quiche vs. neqo-pr: 🚀L1 CM
quinn vs. neqo-pr: V2 CM
s2n-quic vs. neqo-pr: CM
tquic vs. neqo-pr: CM
xquic vs. neqo-pr: M CM
All results

Succeeded Interop Tests

QUIC Interop Runner, client vs. server

neqo-pr as client

neqo-pr as server

Unsupported Interop Tests

QUIC Interop Runner, client vs. server

neqo-pr as client

neqo-pr as server

Merged via the queue into mozilla:main with commit 67ee03f May 28, 2026
183 of 187 checks passed
@larseggert larseggert deleted the feat-has_terminal branch May 28, 2026 07:15
@github-actions
Copy link
Copy Markdown
Contributor

Benchmark results

Significant performance differences relative to 3707580.

streams/walltime/1000-streams/each-1-bytes: 💚 Performance has improved by -1.6993%.
       time:   [11.711 ms 11.726 ms 11.742 ms]
       change: [-1.9147% -1.6993% -1.4773] (p = 0.00 < 0.05)
       Performance has improved.
streams/walltime/1000-streams/each-1000-bytes: 💚 Performance has improved by -10.942%.
       time:   [36.735 ms 36.829 ms 36.959 ms]
       change: [-11.203% -10.942% -10.647] (p = 0.00 < 0.05)
       Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
2 (2.00%) high mild
1 (1.00%) high severe
streams-flow-controlled/walltime/1-streams/each-4194304-bytes: 💚 Performance has improved by -1.5579%.
       time:   [32.390 ms 32.441 ms 32.491 ms]
       change: [-1.9078% -1.5579% -1.2602] (p = 0.00 < 0.05)
       Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) low mild
1 (1.00%) high mild
streams-flow-controlled/walltime/10-streams/each-1048576-bytes: 💚 Performance has improved by -3.7095%.
       time:   [86.874 ms 87.147 ms 87.468 ms]
       change: [-4.2768% -3.7095% -3.1665] (p = 0.00 < 0.05)
       Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
2 (2.00%) low mild
2 (2.00%) high mild
1 (1.00%) high severe
All results
transfer/1-conn/1-100mb-resp (aka. Download): Change within noise threshold.
       time:   [176.21 ms 176.56 ms 176.94 ms]
       thrpt:  [565.16 MiB/s 566.37 MiB/s 567.52 MiB/s]
change:
       time:   [-0.8725% -0.5935% -0.2996] (p = 0.00 < 0.05)
       thrpt:  [+0.3005% +0.5971% +0.8801]
       Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high mild
transfer/1-conn/10_000-parallel-1b-resp (aka. RPS): No change in performance detected.
       time:   [282.59 ms 284.39 ms 286.21 ms]
       thrpt:  [34.940 Kelem/s 35.163 Kelem/s 35.387 Kelem/s]
change:
       time:   [-1.0497% -0.1498% +0.6818] (p = 0.75 > 0.05)
       thrpt:  [-0.6772% +0.1500% +1.0608]
       No change in performance detected.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mild
transfer/1-conn/1-1b-resp (aka. HPS): No change in performance detected.
       time:   [38.387 ms 38.492 ms 38.613 ms]
       thrpt:  [25.898   B/s 25.980   B/s 26.050   B/s]
change:
       time:   [-0.6247% -0.1438% +0.3205] (p = 0.56 > 0.05)
       thrpt:  [-0.3195% +0.1440% +0.6286]
       No change in performance detected.
Found 5 outliers among 100 measurements (5.00%)
3 (3.00%) high mild
2 (2.00%) high severe
transfer/1-conn/1-100mb-req (aka. Upload): Change within noise threshold.
       time:   [180.93 ms 181.32 ms 181.75 ms]
       thrpt:  [550.21 MiB/s 551.53 MiB/s 552.69 MiB/s]
change:
       time:   [+0.1960% +0.4676% +0.7666] (p = 0.00 < 0.05)
       thrpt:  [-0.7607% -0.4654% -0.1956]
       Change within noise threshold.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high severe
streams/walltime/1-streams/each-1000-bytes: No change in performance detected.
       time:   [570.34 µs 572.22 µs 574.45 µs]
       change: [-0.5315% -0.0206% +0.4836] (p = 0.94 > 0.05)
       No change in performance detected.
Found 10 outliers among 100 measurements (10.00%)
3 (3.00%) high mild
7 (7.00%) high severe
streams/walltime/1000-streams/each-1-bytes: 💚 Performance has improved by -1.6993%.
       time:   [11.711 ms 11.726 ms 11.742 ms]
       change: [-1.9147% -1.6993% -1.4773] (p = 0.00 < 0.05)
       Performance has improved.
streams/walltime/1000-streams/each-1000-bytes: 💚 Performance has improved by -10.942%.
       time:   [36.735 ms 36.829 ms 36.959 ms]
       change: [-11.203% -10.942% -10.647] (p = 0.00 < 0.05)
       Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
2 (2.00%) high mild
1 (1.00%) high severe
streams-flow-controlled/walltime/1-streams/each-4194304-bytes: 💚 Performance has improved by -1.5579%.
       time:   [32.390 ms 32.441 ms 32.491 ms]
       change: [-1.9078% -1.5579% -1.2602] (p = 0.00 < 0.05)
       Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) low mild
1 (1.00%) high mild
streams-flow-controlled/walltime/10-streams/each-1048576-bytes: 💚 Performance has improved by -3.7095%.
       time:   [86.874 ms 87.147 ms 87.468 ms]
       change: [-4.2768% -3.7095% -3.1665] (p = 0.00 < 0.05)
       Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
2 (2.00%) low mild
2 (2.00%) high mild
1 (1.00%) high severe
transfer/walltime/pacing-false/varying-seeds: Change within noise threshold.
       time:   [21.115 ms 21.134 ms 21.158 ms]
       change: [-0.3032% -0.1831% -0.0564] (p = 0.00 < 0.05)
       Change within noise threshold.
Found 3 outliers among 100 measurements (3.00%)
2 (2.00%) high mild
1 (1.00%) high severe
transfer/walltime/pacing-true/varying-seeds: Change within noise threshold.
       time:   [21.340 ms 21.365 ms 21.399 ms]
       change: [-2.7226% -2.5794% -2.4094] (p = 0.00 < 0.05)
       Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severe
transfer/walltime/pacing-false/same-seed: Change within noise threshold.
       time:   [20.890 ms 20.907 ms 20.926 ms]
       change: [-0.9149% -0.7316% -0.5794] (p = 0.00 < 0.05)
       Change within noise threshold.
Found 3 outliers among 100 measurements (3.00%)
2 (2.00%) high mild
1 (1.00%) high severe
transfer/walltime/pacing-true/same-seed: Change within noise threshold.
       time:   [21.659 ms 21.675 ms 21.691 ms]
       change: [-0.9794% -0.8500% -0.7277] (p = 0.00 < 0.05)
       Change within noise threshold.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mild

Download data for profiler.firefox.com or download performance comparison data.

@github-actions
Copy link
Copy Markdown
Contributor

Client/server transfer results

Performance differences relative to 3707580.

Transfer of 33554432 bytes over loopback, min. 100 runs. All unit-less numbers are in milliseconds.

Client vs. server (params) Mean ± σ Min Max MiB/s ± σ Δ baseline Δ baseline
google-neqo-cubic 264.4 ± 2.2 258.3 269.3 121.0 ± 14.5 💔 0.6 0.2%
neqo-google-cubic 770.3 ± 3.2 764.6 781.7 41.5 ± 10.0 💔 0.9 0.1%

Table above only shows statistically significant changes. See all results below.

All results

Transfer of 33554432 bytes over loopback, min. 100 runs. All unit-less numbers are in milliseconds.

Client vs. server (params) Mean ± σ Min Max MiB/s ± σ Δ baseline Δ baseline
google-google-nopacing 463.0 ± 2.2 457.9 476.8 69.1 ± 14.5
google-neqo-cubic 264.4 ± 2.2 258.3 269.3 121.0 ± 14.5 💔 0.6 0.2%
msquic-msquic-nopacing 137.5 ± 118.3 110.5 1296.7 232.8 ± 0.3
msquic-neqo-cubic 157.2 ± 40.8 120.4 419.0 203.5 ± 0.8 -7.8 -4.7%
neqo-google-cubic 770.3 ± 3.2 764.6 781.7 41.5 ± 10.0 💔 0.9 0.1%
neqo-msquic-cubic 146.0 ± 1.6 143.7 151.4 219.2 ± 20.0 0.2 0.1%
neqo-neqo-cubic 84.4 ± 2.8 78.0 94.4 379.2 ± 11.4 0.0 0.0%
neqo-neqo-cubic-nopacing 83.5 ± 2.9 77.7 90.8 383.3 ± 11.0 -0.0 -0.0%
neqo-neqo-newreno 84.5 ± 2.7 79.5 90.7 378.5 ± 11.9 0.3 0.4%
neqo-neqo-newreno-nopacing 83.9 ± 2.8 76.5 92.9 381.5 ± 11.4 0.1 0.1%
neqo-quiche-cubic 192.0 ± 2.1 187.6 197.8 166.7 ± 15.2 0.5 0.3%
neqo-s2n-cubic 215.2 ± 1.3 210.7 221.6 148.7 ± 24.6 0.2 0.1%
quiche-neqo-cubic 180.5 ± 3.7 174.1 194.9 177.3 ± 8.6 0.5 0.3%
quiche-quiche-nopacing 139.9 ± 3.2 133.2 149.5 228.7 ± 10.0
s2n-neqo-cubic 215.2 ± 4.4 207.9 228.6 148.7 ± 7.3 0.4 0.2%
s2n-s2n-nopacing 289.6 ± 17.3 279.6 388.0 110.5 ± 1.8

Download data for profiler.firefox.com or download performance comparison data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants