perf(encode): stop re-reading the environment on every submit and poll

`std::env::var` was on three per-frame paths. Measured on `.173` (Windows, 57
environment variables, 2M iterations): **121.9 ns** per call for the NVENC
in-flight cap and **114.9 ns** per call for the ffmpeg poll spin, against
**0.9 ns** once memoized. On Linux, 32 ns → 1.5 ns.

⚠ Those numbers deflate the filing, and that is worth recording: the audit
ranked this as a hot-path defect, but ~120 ns/frame is ~0.003% of a frame
budget. The fix is still right — it is free, and it takes a global environment
lock off the encode thread — but nobody should schedule it ahead of anything on
the strength of the "hot path" framing.

The severity RANKING was also inverted, and the measurement confirms why. The
site the audit called worst — `nvenc_cuda`'s backpressure loop condition — costs
a default session nothing, because the condition short-circuits on
`async_rt.is_some()` and the default session never engages the two-thread
retrieve. The one that actually pays every frame is Windows `submit`, which
consults `async_inflight_cap()` in BOTH arms of the ring-depth match, sync mode
included, where the result is then thrown away. Windows `poll` is the second
unconditional one, and the audit ranked it last.

Memoized inside each helper rather than latched into a session field. Nothing in
the workspace mutates these variables at runtime (enumerated: no `set_var` for
either key anywhere in first-party code, and the Windows service's arbitrary-key
`host.env` loader runs in the SCM supervisor, which re-execs the host as a child
and never opens an encoder itself). A field would instead change WHEN the value
is read — and the Windows `cap` composes the env with `input_ring_depth`, which
`set_input_ring_depth` may change after open, so freezing that half would
reintroduce the in-place-overwrite bug the ring term exists to prevent.

Two deliberate behaviour changes ride along on `PUNKTFUNK_FFWIN_POLL_MS`, so
"behaviour-preserving" describes the memoization only, not this whole commit:

- **A 1000 ms ceiling.** The reachable hazard was never the overflow — that
  needed `ms >= 1.8e16` — it was a slipped digit: `=100000000` was a 27.7-hour
  spin of the encode thread.
- **`.trim()`, now on all three parsers.** An earlier draft applied the house
  rule (WP7.8) to one of the three, which left `PUNKTFUNK_NVENC_ASYNC=" 1 "`
  working while `PUNKTFUNK_NVENC_ASYNC_DEPTH=" 6 "` silently fell back to 4 —
  and memoization would have frozen that silent fallback for the process
  lifetime. Reachable: the Windows `host.env` loader trims around `=` before
  stripping quotes, so `=" 2 "` yields a value with inner spaces.

⚠ Correction to an earlier draft of this message, which asserted that the audit's
`saturating_mul` proposal would relocate an overflow panic into release builds.
That is FALSE and the code comment now says so: `Duration::from_micros(u64::MAX)`
is ~1.8e13 seconds, six orders of magnitude below `Duration`'s ceiling, so
`Instant + Duration` neither overflows nor panics (measured, with and without
debug assertions). `saturating_mul` is still wrong, for a different reason — it
sets a deadline ~584,000 years out on a spin that provably never produces the
owed AU, i.e. it wedges the encode thread permanently. A hang, not a panic.

WP6.1.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-07-25 17:55:59 +02:00
co-authored by Claude Opus 5
parent 0044649b19
commit e680096c6a
3 changed files with 68 additions and 16 deletions
+8 -2
View File
@@ -269,13 +269,19 @@ fn async_retrieve_requested() -> bool {
/// Max encodes in flight in two-thread mode (`PUNKTFUNK_NVENC_ASYNC_DEPTH`, default 4, clamped /// Max encodes in flight in two-thread mode (`PUNKTFUNK_NVENC_ASYNC_DEPTH`, default 4, clamped
/// `2..=POOL-1` — a bitstream must never be reused mid-encode, and the input ring is the same /// `2..=POOL-1` — a bitstream must never be reused mid-encode, and the input ring is the same
/// depth). Mirrors the Windows knob exactly. /// depth). Mirrors the Windows knob exactly, memoization included: this is the backpressure
/// **loop condition** in `submit`, so an engaged two-thread session re-read the environment once
/// per spin. The default session never pays it (the condition short-circuits on `async_rt`), which
/// is why the audit's severity ranking for this site was inverted — but an escalated one did.
fn async_inflight_cap() -> usize { fn async_inflight_cap() -> usize {
static CAP: std::sync::OnceLock<usize> = std::sync::OnceLock::new();
*CAP.get_or_init(|| {
std::env::var("PUNKTFUNK_NVENC_ASYNC_DEPTH") std::env::var("PUNKTFUNK_NVENC_ASYNC_DEPTH")
.ok() .ok()
.and_then(|s| s.parse::<usize>().ok()) .and_then(|s| s.trim().parse::<usize>().ok())
.unwrap_or(4) .unwrap_or(4)
.clamp(2, POOL - 1) .clamp(2, POOL - 1)
})
} }
/// Stream-ordered submit (`PUNKTFUNK_NVENC_STREAM_ORDERED`, default ON; `0` = the pre-existing /// Stream-ordered submit (`PUNKTFUNK_NVENC_STREAM_ORDERED`, default ON; `0` = the pre-existing
+39 -5
View File
@@ -134,6 +134,44 @@ fn zerocopy_enabled(vendor: WinVendor) -> bool {
.unwrap_or(matches!(vendor, WinVendor::Amf)) .unwrap_or(matches!(vendor, WinVendor::Amf))
} }
/// Upper bound on `PUNKTFUNK_FFWIN_POLL_MS`. This knob spins the **encode thread** waiting for an
/// AU, so a value past one frame period is already self-defeating and a full second is far beyond
/// anything an operator would set on purpose. The clamp is also what makes the µs conversion
/// below provably overflow-free.
///
/// The reachable hazard is a slipped digit, not the overflow: pre-clamp, `PUNKTFUNK_FFWIN_POLL_MS=
/// 100000000` was a **27.7-hour** spin with no overflow anywhere near it.
const MAX_POLL_SPIN_MS: u64 = 1_000;
/// Bounded post-submit spin for [`FfmpegWinEncoder::poll`], in microseconds (0 = off, the default
/// and the correct choice on every VCN measured so far).
///
/// Read from the environment **once per process** (WP6.1): `poll` runs once per encode tick, and
/// this was an unconditional `env::var` + parse on it.
///
/// ⚠ The audit proposed `saturating_mul` for the µs conversion. It is still the wrong fix, but for
/// a reason worth stating precisely, because the obvious one is false: `Duration::from_micros(
/// u64::MAX)` is only ~1.8e13 seconds, six orders of magnitude below `Duration`'s `u64::MAX`-second
/// ceiling, so `Instant::now() + Duration::from_micros(u64::MAX)` does **not** overflow and does
/// **not** panic (measured, both with and without debug assertions). What it does instead is set a
/// deadline ~584,000 years out, and the loop below only exits on `Packet`/`Eof` — and this
/// function's own doc explains that a spin here *provably never* produces the owed AU on the
/// measured hardware. So `saturating_mul` converts a bad value into a **permanently wedged encode
/// thread**: a hang, not a panic. Clamping the parsed value first removes the bad value entirely.
///
/// (For the record on the pre-clamp behaviour: the workspace sets no `overflow-checks` in
/// `[profile.release]`, so `ms * 1000` wrapped silently in release and panicked only in debug.)
fn poll_spin_cap_us() -> u64 {
static CAP_US: std::sync::OnceLock<u64> = std::sync::OnceLock::new();
*CAP_US.get_or_init(|| {
std::env::var("PUNKTFUNK_FFWIN_POLL_MS")
.ok()
.and_then(|s| s.trim().parse::<u64>().ok())
.map(|ms| ms.min(MAX_POLL_SPIN_MS) * 1000)
.unwrap_or(0) // default: no spin — the libavcodec AMF buffer can't be spun out
})
}
/// The swscale *source* pixel format for a captured packed-RGB/BGR layout (8-bit BGRA fallback only). /// The swscale *source* pixel format for a captured packed-RGB/BGR layout (8-bit BGRA fallback only).
fn sws_src(format: PixelFormat) -> Result<Pixel> { fn sws_src(format: PixelFormat) -> Result<Pixel> {
Ok(match format { Ok(match format {
@@ -1332,11 +1370,7 @@ impl Encoder for FfmpegWinEncoder {
Some(Inner::ZeroCopy(z)) => &mut z.enc, Some(Inner::ZeroCopy(z)) => &mut z.enc,
None => return Ok(None), None => return Ok(None),
}; };
let cap_us = std::env::var("PUNKTFUNK_FFWIN_POLL_MS") let cap_us = poll_spin_cap_us();
.ok()
.and_then(|s| s.parse::<u64>().ok())
.map(|ms| ms * 1000)
.unwrap_or(0); // default: no spin — the libavcodec AMF buffer can't be spun out
let deadline = (cap_us > 0 && self.in_flight > 0) let deadline = (cap_us > 0 && self.in_flight > 0)
.then(|| std::time::Instant::now() + std::time::Duration::from_micros(cap_us)); .then(|| std::time::Instant::now() + std::time::Duration::from_micros(cap_us));
loop { loop {
+13 -1
View File
@@ -386,12 +386,24 @@ fn async_retrieve_requested() -> bool {
/// the ring textures in place, so in-flight depth beyond the ring lets the capturer overwrite a /// the ring textures in place, so in-flight depth beyond the ring lets the capturer overwrite a
/// frame mid-encode: visual corruption, not UB). IDD-push rings are sized around /// frame mid-encode: visual corruption, not UB). IDD-push rings are sized around
/// `PUNKTFUNK_IDD_DEPTH`; raise both together if deeper pipelining is needed. /// `PUNKTFUNK_IDD_DEPTH`; raise both together if deeper pipelining is needed.
///
/// Read from the environment **once per process** (WP6.1): `submit` consults this on EVERY frame —
/// both arms of the `cap` match, in sync mode too, where the result is then discarded because the
/// backpressure loop short-circuits on `async_rt`. Memoizing rather than latching into a session
/// field is deliberate: the composed `cap` also folds in `input_ring_depth`, which
/// `set_input_ring_depth` may change after open, and freezing that half would reintroduce the
/// in-place-overwrite bug the ring term exists to prevent. Nothing in the workspace mutates this
/// variable at runtime, so memoizing a process-constant read is behaviour-preserving by
/// construction.
fn async_inflight_cap() -> usize { fn async_inflight_cap() -> usize {
static CAP: std::sync::OnceLock<usize> = std::sync::OnceLock::new();
*CAP.get_or_init(|| {
std::env::var("PUNKTFUNK_NVENC_ASYNC_DEPTH") std::env::var("PUNKTFUNK_NVENC_ASYNC_DEPTH")
.ok() .ok()
.and_then(|s| s.parse::<usize>().ok()) .and_then(|s| s.trim().parse::<usize>().ok())
.unwrap_or(4) .unwrap_or(4)
.clamp(2, POOL - 1) .clamp(2, POOL - 1)
})
} }
/// One in-flight encode handed to the retrieve thread: the output bitstream to lock once its /// One in-flight encode handed to the retrieve thread: the output bitstream to lock once its