Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: Compare performance to msquic #1750

Merged
merged 53 commits into from
Mar 25, 2024
Merged

Conversation

larseggert
Copy link
Collaborator

In progress

Copy link

github-actions bot commented Mar 15, 2024

Benchmark results

Performance differences relative to 50876af.

  • drain a timer quickly time: [391.85 ns 398.79 ns 405.11 ns]
    change: [-3.5688% -2.0497% -0.5018%] (p = 0.01 < 0.05)
    Change within noise threshold.

  • coalesce_acked_from_zero 1+1 entries
    time: [193.77 ns 194.19 ns 194.66 ns]
    change: [-0.9878% -0.3625% +0.2183%] (p = 0.25 > 0.05)
    No change in performance detected.

  • coalesce_acked_from_zero 3+1 entries
    time: [235.95 ns 236.50 ns 237.09 ns]
    change: [-0.4647% -0.0168% +0.4699%] (p = 0.94 > 0.05)
    No change in performance detected.

  • coalesce_acked_from_zero 10+1 entries
    time: [236.81 ns 237.90 ns 239.06 ns]
    change: [-0.4510% +0.1024% +0.6451%] (p = 0.72 > 0.05)
    No change in performance detected.

  • coalesce_acked_from_zero 1000+1 entries
    time: [214.16 ns 214.39 ns 214.69 ns]
    change: [-1.0684% -0.3058% +0.5524%] (p = 0.46 > 0.05)
    No change in performance detected.

  • RxStreamOrderer::inbound_frame()
    time: [117.34 ms 117.43 ms 117.52 ms]
    change: [-1.2006% -1.0891% -0.9835%] (p = 0.00 < 0.05)
    Change within noise threshold.

  • transfer/Run multiple transfers with varying seeds
    time: [118.14 ms 118.41 ms 118.67 ms]
    thrpt: [33.707 MiB/s 33.782 MiB/s 33.858 MiB/s]
    change:
    time: [+0.8201% +1.1651% +1.5168%] (p = 0.00 < 0.05)
    thrpt: [-1.4941% -1.1517% -0.8134%]
    Change within noise threshold.

  • transfer/Run multiple transfers with the same seed
    time: [119.00 ms 119.16 ms 119.33 ms]
    thrpt: [33.520 MiB/s 33.567 MiB/s 33.613 MiB/s]
    change:
    time: [+0.8034% +0.9808% +1.1608%] (p = 0.00 < 0.05)
    thrpt: [-1.1475% -0.9712% -0.7970%]
    Change within noise threshold.

Client/server transfer results

Transfer of 134217728 bytes over loopback.

Client Server CC Pacing Mean [ms] Min [ms] Max [ms] Relative
msquic msquic 408.4 ± 62.6 350.9 513.9 1.00
neqo msquic reno on 1912.2 ± 27.1 1879.0 1959.9 1.00
neqo msquic reno 1899.1 ± 41.2 1826.5 1970.3 1.00
neqo msquic cubic on 1908.7 ± 47.5 1858.6 2001.2 1.00
neqo msquic cubic 1923.9 ± 56.6 1859.3 2067.1 1.00
msquic neqo reno on 4366.5 ± 148.7 4157.5 4598.2 1.00
msquic neqo reno 4303.2 ± 144.3 4108.4 4542.4 1.00
msquic neqo cubic on 4463.2 ± 161.3 4226.4 4664.8 1.00
msquic neqo cubic 4379.4 ± 169.1 4156.6 4643.9 1.00
neqo neqo reno on 3616.2 ± 103.1 3418.1 3721.7 1.00
neqo neqo reno 3457.4 ± 175.1 3210.3 3731.3 1.00
neqo neqo cubic on 4214.0 ± 146.4 3979.5 4415.5 1.00
neqo neqo cubic 4192.3 ± 159.8 3915.8 4396.9 1.00

⬇️ Download logs

Copy link

codecov bot commented Mar 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.11%. Comparing base (50876af) to head (3798f0b).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1750      +/-   ##
==========================================
+ Coverage   93.08%   93.11%   +0.02%     
==========================================
  Files         117      117              
  Lines       36422    36422              
==========================================
+ Hits        33903    33913      +10     
+ Misses       2519     2509      -10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

commit 9ca5ecc
Merge: f50f414 f408321
Author: Lars Eggert <[email protected]>
Date:   Fri Mar 15 00:12:29 2024 +0200

    Merge branch 'main' into ci-bench-cc

    Signed-off-by: Lars Eggert <[email protected]>

commit f50f414
Merge: 8e5290b bc262a5
Author: Lars Eggert <[email protected]>
Date:   Wed Mar 13 07:59:01 2024 +0200

    Merge branch 'main' into ci-bench-cc

commit 8e5290b
Merge: f0cd19e 2ff9742
Author: Lars Eggert <[email protected]>
Date:   Tue Mar 12 22:42:54 2024 +0200

    Merge branch 'main' into ci-bench-cc

commit f0cd19e
Merge: b2bb855 17c4175
Author: Lars Eggert <[email protected]>
Date:   Tue Mar 12 21:54:08 2024 +0200

    Merge branch 'main' into ci-bench-cc

commit b2bb855
Merge: d072504 4ea2c56
Author: Lars Eggert <[email protected]>
Date:   Tue Mar 12 17:25:13 2024 +0200

    Merge branch 'ci-bench-cc' of github.com:larseggert/neqo into ci-bench-cc

commit d072504
Author: Lars Eggert <[email protected]>
Date:   Tue Mar 12 17:24:52 2024 +0200

    Reorder things so `results.ms` is included in the exported artifact

commit 4ea2c56
Merge: c82ff3a 5c72890
Author: Lars Eggert <[email protected]>
Date:   Tue Mar 12 17:18:37 2024 +0200

    Merge branch 'main' into ci-bench-cc

commit c82ff3a
Author: Lars Eggert <[email protected]>
Date:   Tue Mar 12 16:41:59 2024 +0200

    `killall` -> `pkill`

commit d37e706
Author: Lars Eggert <[email protected]>
Date:   Tue Mar 12 16:37:50 2024 +0200

    Go back to `killall`

commit 11320d0
Author: Lars Eggert <[email protected]>
Date:   Tue Mar 12 16:11:38 2024 +0200

    No -INT

commit 407bd4f
Author: Lars Eggert <[email protected]>
Date:   Tue Mar 12 14:33:52 2024 +0200

    kill -> killall

    Also reduce test transfer size.

commit 9d3a8b7
Author: Lars Eggert <[email protected]>
Date:   Tue Mar 12 13:57:51 2024 +0200

    Use temp dir, and fix path error

commit 84e2206
Merge: 925cc12 b0d816a
Author: Lars Eggert <[email protected]>
Date:   Tue Mar 12 11:10:41 2024 +0200

    Merge branch 'main' into ci-bench-cc

commit 925cc12
Merge: 3241f93 5889038
Author: Lars Eggert <[email protected]>
Date:   Tue Mar 12 11:05:42 2024 +0200

    Merge branch 'main' into ci-bench-cc

commit 3241f93
Merge: 02620a7 d48fbed
Author: Lars Eggert <[email protected]>
Date:   Tue Mar 12 09:59:24 2024 +0200

    Merge branch 'main' into ci-bench-cc

    Signed-off-by: Lars Eggert <[email protected]>

commit 02620a7
Author: Lars Eggert <[email protected]>
Date:   Tue Mar 12 09:57:33 2024 +0200

    Try to kill via `$!`

commit b32ce9e
Merge: 9ea3a99 db1dbb2
Author: Lars Eggert <[email protected]>
Date:   Tue Mar 12 09:15:18 2024 +0200

    Merge branch 'ci-bench-cc' of github.com:larseggert/neqo into ci-bench-cc

commit 9ea3a99
Author: Lars Eggert <[email protected]>
Date:   Tue Mar 12 09:15:05 2024 +0200

    Address comments from @martinthomson

commit db1dbb2
Merge: 681bbb7 869afea
Author: Lars Eggert <[email protected]>
Date:   Mon Mar 11 19:33:53 2024 +0200

    Merge branch 'main' into ci-bench-cc

commit 681bbb7
Merge: bd742af 532dcc5
Author: Lars Eggert <[email protected]>
Date:   Mon Mar 11 18:21:06 2024 +0200

    Merge branch 'main' into ci-bench-cc

    Signed-off-by: Lars Eggert <[email protected]>

commit bd742af
Author: Lars Eggert <[email protected]>
Date:   Mon Mar 11 17:00:34 2024 +0200

    mkdir -p

commit bc7b99f
Author: Lars Eggert <[email protected]>
Date:   Mon Mar 11 16:29:14 2024 +0200

    Fix

commit e7bf509
Merge: de64b3e cbd4441
Author: Lars Eggert <[email protected]>
Date:   Mon Mar 11 16:27:56 2024 +0200

    Merge branch 'main' into ci-bench-cc

commit de64b3e
Author: Lars Eggert <[email protected]>
Date:   Mon Mar 11 16:00:19 2024 +0200

    Wait for output before continuing

commit 12386a3
Author: Lars Eggert <[email protected]>
Date:   Mon Mar 11 15:25:40 2024 +0200

    ci: Benchmark NewReno and Cubic
@larseggert larseggert marked this pull request as ready for review March 18, 2024 04:39
@larseggert
Copy link
Collaborator Author

I looked at why we're so much slower then msquic, and the neqo server spends an insane amount of time in neqo-common::timer:

       │      /// Take the next item, unless there are no items with                                                   ▒
       │      /// a timeout in the past relative to `until`.                                                           ▒
       │      pub fn take_next(&mut self, until: Instant) -> Option<T> {                                               ▒
       │      for i in 0..self.items.len() {                                                                           ▒
       │      let idx = self.bucket(i);                                                                                ▒
       │      if !self.items[idx].is_empty() && self.items[idx][0].time <= until {                                     ▒
       │        test      %rbx,%rbx                                                                                    ▒
       │        jne       2444                                                                                         ▒
       │        jmp       23f0                                                                                         ▒
       │      neqo_common::timer::Timer<T>::bucket:                                                                    ▒
       │        nop                                                                                                    ▒
       │      (self.cursor + delta) % self.items.len()                                                                 ▒
  0.03 │2430:   mov       %ecx,%eax                                                                                    ▒
       │        xor       %edx,%edx                                                                                    ▒
 29.50 │        div       %esi                                                                                         ▒
       │      <usize as core::slice::index::SliceIndex<[T]>>::index:                                                   ▒
  1.27 │        lea       (%rdx,%rdx,2),%rax                                                                           ▒
       │      alloc::vec::Vec<T,A>::len:                                                                               ▒
 22.02 │        mov       0x10(%r8,%rax,8),%rbx                                                                        ▒
       │      neqo_common::timer::Timer<T>::take_next:                                                                 ▒
       │      if !self.items[idx].is_empty() && self.items[idx][0].time <= until {                                     ▒
  0.04 │        test      %rbx,%rbx                                                                                    ▒
  7.64 │        je        23f0                                                                                         ▒
       │      neqo_common::timer::Timer<T>::bucket:                                                                    ▒
      │      (self.cursor + delta) % self.items.len()                                                                 ▒
 0.11 │2bf0:   mov       %ecx,%eax                                                                                    ▒
 0.09 │        xor       %edx,%edx                                                                                    ▒
 8.55 │        div       %edi                                                                                         ▒
      │      <usize as core::slice::index::SliceIndex<[T]>>::index:                                                   ▒
 0.12 │        lea       (%rdx,%rdx,2),%rax                                                                           ▒
      │      core::slice::<impl [T]>::first:                                                                          ▒
 4.36 │        inc       %rcx                                                                                         ▒
 4.53 │        cmpq      $0x0,0x10(%rsi,%rax,8)                                                                       ▒
 2.20 │        jne       2c32                                                                                         ▒

(Numbers in the first column are percent of total cycles spent in that line.)

@larseggert larseggert merged commit 76630a5 into mozilla:main Mar 25, 2024
16 checks passed
@larseggert larseggert deleted the ci-bench-msquic branch March 25, 2024 11:33
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.

3 participants