Skip to content

feat(proto): expose max_transmit_segments and max_transmit_datagrams in TransportConfig#645

Open
poka-IT wants to merge 1 commit inton0-computer:mainfrom
poka-IT:warren-perf-tier1-setter-api
Open

feat(proto): expose max_transmit_segments and max_transmit_datagrams in TransportConfig#645
poka-IT wants to merge 1 commit inton0-computer:mainfrom
poka-IT:warren-perf-tier1-setter-api

Conversation

@poka-IT
Copy link
Copy Markdown

@poka-IT poka-IT commented May 8, 2026

Description

Replace the hardcoded constants MAX_TRANSMIT_SEGMENTS = 10 and MAX_TRANSMIT_DATAGRAMS = 20 in noq::Connection::drive_transmit with two configurable fields on TransportConfig. Defaults are unchanged (10 / 20), so default behavior is preserved.

New API:

let mut t = TransportConfig::default();
t.max_transmit_segments(NonZeroUsize::new(40).unwrap());
t.max_transmit_datagrams(NonZeroUsize::new(80).unwrap());

This is an alternative to #636 (which simply bumps the constants to 40 / 80 globally). After feedback from @flub there, exposing the values as a runtime knob seems more defensible:

  • No default-behavior change: multi-connection workloads keep their per-connection memory footprint.
  • Single-connection bulk-transfer workloads opt in: applications such as ours (peer-to-peer VPN tunnel over QUIC) can call transport.max_transmit_segments(NonZeroUsize::new(40).unwrap()) and get the throughput / CPU-efficiency gains demonstrated in the perf(connection): raise MAX_TRANSMIT_SEGMENTS to 40 and MAX_TRANSMIT_DATAGRAMS to 80 #636 benches.
  • Pattern-consistent: same shape as enable_segmentation_offload, congestion_controller_factory, etc.

Why a setter rather than a higher default

MAX_TRANSMIT_SEGMENTS = 10 is exactly equal to MIN_BURST_SIZE in noq-proto/src/connection/pacing.rs:162, the historical 10 was likely chosen to match the pacer's minimum burst. The pacer itself, however, already allows up to MAX_BURST_SIZE = 256 per burst, so even a setter value of 256 stays within the existing pacer envelope. CC and pacing remain authoritative; the constants are just an outer cap.

Trade-offs now self-documenting

Breaking Changes

n/a, defaults preserved (10 / 20).

Notes & open questions

Bench data motivating both PRs (paired same-session, 3x CCX23 Hetzner, real network: intra-DC and inter-region EU) is in the discussion thread of #636.

Change checklist

  • Self-review.
  • Tests (defaults preserved + setter behavior).
  • No breaking changes.

@poka-IT poka-IT force-pushed the warren-perf-tier1-setter-api branch from f0f183c to ade217c Compare May 8, 2026 03:44
@n0bot n0bot Bot added this to iroh May 8, 2026
@github-project-automation github-project-automation Bot moved this to 🚑 Needs Triage in iroh May 8, 2026
@dignifiedquire
Copy link
Copy Markdown
Contributor

I like this direction, as it reflects reality better, that these values need to be different depending on actual usage.

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

Labels

None yet

Projects

Status: 🚑 Needs Triage

Development

Successfully merging this pull request may close these issues.

2 participants