fix(bin): don't sleep when events available

[mxinden]

A call to process_* can emit events as a side effect, e.g. a ConnectionEvent::StateChange(State::Connected). These are not returned from the function call, but instead internally enqueued to be later on consumed through the various Provider::next_event implementations.

neqo/neqo-common/src/event.rs

Lines 9 to 15 in 166b84c

	/// An event provider is able to generate a stream of events.
	pub trait Provider {
	type Event;
	/// Get the next event.
	#[must_use]
	fn next_event(&mut self) -> Option<Self::Event>;

In the case of neqo-client the events are consumed through self.handler.handle which internally calls Provider::next_event.

A client or server should not go to sleep, waiting for either a UDP datagram to arrive or a timeout to fire, when there are events available.

This commit ensures ready().await is only called when no events are available.

---

Extracted from #1741.

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 93.12%. Comparing base (166b84c) to head (342f492).

Files	Patch %	Lines
neqo-transport/src/server.rs	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1806      +/-   ##
==========================================
- Coverage   93.13%   93.12%   -0.01%     
==========================================
  Files         117      117              
  Lines       36363    36366       +3     
==========================================
  Hits        33865    33865              
- Misses       2498     2501       +3     

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

[github-actions]

Benchmark results

Performance differences relative to c004359.

• drain a timer quickly time: [421.56 ns 427.86 ns 433.69 ns]
  change: [-1.4181% -0.0938% +1.2854%] (p = 0.90 > 0.05)
  No change in performance detected.

• coalesce_acked_from_zero 1+1 entries
  time: [200.01 ns 200.49 ns 201.01 ns]
  change: [-0.5788% -0.0099% +0.6159%] (p = 0.98 > 0.05)
  No change in performance detected.

• coalesce_acked_from_zero 3+1 entries
  time: [241.76 ns 242.37 ns 243.00 ns]
  change: [+0.0634% +0.3464% +0.6351%] (p = 0.02 < 0.05)
  Change within noise threshold.

• coalesce_acked_from_zero 10+1 entries
  time: [240.94 ns 244.92 ns 252.78 ns]
  change: [-0.1712% +0.9855% +2.7959%] (p = 0.26 > 0.05)
  No change in performance detected.

• coalesce_acked_from_zero 1000+1 entries
  time: [219.97 ns 220.20 ns 220.45 ns]
  change: [-13.818% -4.8740% +0.6441%] (p = 0.46 > 0.05)
  No change in performance detected.

• RxStreamOrderer::inbound_frame()
  time: [118.98 ms 119.14 ms 119.38 ms]
  change: [+0.1926% +0.3633% +0.5809%] (p = 0.00 < 0.05)
  Change within noise threshold.

• transfer/Run multiple transfers with varying seeds
  time: [122.94 ms 123.22 ms 123.49 ms]
  thrpt: [32.391 MiB/s 32.464 MiB/s 32.536 MiB/s]
  change:
  time: [+2.3660% +2.7033% +3.0244%] (p = 0.00 < 0.05)
  thrpt: [-2.9356% -2.6321% -2.3113%]
  Change within noise threshold.

• transfer/Run multiple transfers with the same seed
  time: [123.90 ms 124.09 ms 124.30 ms]
  thrpt: [32.181 MiB/s 32.234 MiB/s 32.283 MiB/s]
  change:
  time: [+2.5327% +2.7517% +2.9943%] (p = 0.00 < 0.05)
  thrpt: [-2.9073% -2.6780% -2.4701%]
  Change within noise threshold.

• 1-conn/1-100mb-resp (aka. Download)/client
  time: [1.1298 s 1.1329 s 1.1362 s]
  thrpt: [88.016 MiB/s 88.266 MiB/s 88.509 MiB/s]
  change:
  time: [+3.8734% +4.7444% +5.4991%] (p = 0.00 < 0.05)
  thrpt: [-5.2124% -4.5295% -3.7289%]
  💔 Performance has regressed.

• 1-conn/10_000-parallel-1b-resp (aka. RPS)/client
  time: [425.36 ms 427.62 ms 429.95 ms]
  thrpt: [23.259 Kelem/s 23.385 Kelem/s 23.510 Kelem/s]
  change:
  time: [-0.8187% -0.1275% +0.6005%] (p = 0.73 > 0.05)
  thrpt: [-0.5969% +0.1277% +0.8254%]
  No change in performance detected.

• 1-conn/1-1b-resp (aka. HPS)/client
  time: [50.494 ms 50.821 ms 51.176 ms]
  thrpt: [19.541 elem/s 19.677 elem/s 19.804 elem/s]
  change:
  time: [-3.9379% -2.9303% -1.9045%] (p = 0.00 < 0.05)
  thrpt: [+1.9415% +3.0187% +4.0994%]
  💚 Performance has improved.

Client/server transfer results

Transfer of 134217728 bytes over loopback.

Client	Server	CC	Pacing	Mean [ms]	Min [ms]	Max [ms]	Relative
msquic	msquic			710.1 ± 194.7	429.1	1031.9	1.00
neqo	msquic	reno	on	1135.4 ± 386.4	831.3	1839.6	1.00
neqo	msquic	reno		1178.8 ± 237.7	811.7	1476.4	1.00
neqo	msquic	cubic	on	1198.7 ± 311.1	840.7	1668.1	1.00
neqo	msquic	cubic		1064.3 ± 312.3	813.0	1765.0	1.00
msquic	neqo	reno	on	4603.5 ± 265.4	4214.9	5126.1	1.00
msquic	neqo	reno		4489.3 ± 284.0	4089.6	4995.0	1.00
msquic	neqo	cubic	on	4534.7 ± 264.5	4203.5	4958.9	1.00
msquic	neqo	cubic		4450.2 ± 182.3	4152.6	4759.2	1.00
neqo	neqo	reno	on	3604.7 ± 210.1	3239.5	3961.9	1.00
neqo	neqo	reno		3684.5 ± 369.0	3260.9	4363.2	1.00
neqo	neqo	cubic	on	4300.8 ± 281.7	3946.1	4772.1	1.00
neqo	neqo	cubic		4380.0 ± 347.7	4037.4	5136.6	1.00

⬇️ Download logs

[larseggert]

Could we add a test for servers.rs?

Also see #921 in case this is something that might affect things here.

[mxinden]

Also see #921 in case this is something that might affect things here.

Given that Server::active_connections always takes all of self.active (mem::take), I don't think the change proposed in this pull request is affected by #921. I created #1818 to test whether the potential duplication in Http3Server::process is an issue in general. Let's see what the benchmarks say.

---

Could we add a test for servers.rs?

Done with the latest commit. @larseggert mind taking another look?