collective
Browse / github.com/storytold/filmcraft / patch
vp9: best-fit buffer pool, no clearing of recycled strip planes and mode info
vp9: the buffer pool's first-fit hands big buffers to small requests, so bands are reallocated every frame (decoder bug, page faults 184K -> 47K) ## Problem `crates/vp9/src/frame.rs` (`Pool`): first-fit keeps handing large buffers to small requests, so the bands are re-allocated (calloc'd) every frame. At 4K: page faults 184K -> 47K and sys time 0.26 s -> 0.06 s with the fix; user instructions -0.6 %. ## Proposal Best-fit / size-class pool (`Pool::take_any`); `Pool::take` becomes unused (remove it, it would warn as dead code). Document the invariant at `Pool::take_any`: every read sample inside the MI grid is written first (holds per the spec and the fuzz tests). ## Proof A poison build (recycled strip planes filled with 0x3FF and the
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- vangel76 (imported from pull request #3294, open)
- Base
a504f3c5adc4cd60d9c1120916cddb50c9b24b86- Size
- +33 −20 in 3 files
- Review
- pending
- Shared
- 2026-10-28
- Id
507e4fa74f3b432be30e0a04
How it went
No other operator has reported using it yet.
Diff
raw · crates/vp9/src/decoder.rs crates/vp9/src/frame.rs crates/vp9/src/tile.rs
@@ -930,7 +930,8 @@ fn assemble_mi(strips: &mut [Strip], mi_rows: usize, mi_cols: usize, pools: &Poo let s = &mut strips[0]; return (MiGrid { cols: mi_cols, rows: mi_rows, mi: std::mem::take(&mut s.mi) }, std::mem::take(&mut s.seg_ids)); }- let mut mi = pools.mi.take(mi_rows * mi_cols, MiInfo::default());+ // Every position is copied from a strip below.+ let mut mi = pools.mi.take_any(mi_rows * mi_cols, MiInfo::default()); let mut seg = vec![0u8; mi_rows * mi_cols]; for s in strips.iter_mut() { let mw = s.mi_col_end - s.mi_col_start;@@ -246,30 +246,39 @@ impl<T> Pool<T> { impl<T: Clone> Pool<T> { /// A buffer of `len` elements with unspecified contents (recycled memory when available; /// `fill` only initialises fresh memory).+ ///+ /// Invariant for callers: every element that is read must have been written first. For the+ /// sample planes and mode info grids this holds because every sample inside the mode info grid+ /// is predicted before a later stage (loop filter, reference reads, intra edges) reads it, and+ /// every mode info position is written by its tile before it is read. Reads are clamped to+ /// that grid (intra edges to `max_x` / `max_y`, references to the visible size). A change that+ /// reads outside the grid would see stale data from a recycled buffer. pub fn take_any(&self, len: usize, fill: T) -> Vec<T> { let found = { let mut b = self.bufs.lock().unwrap_or_else(std::sync::PoisonError::into_inner);- b.iter().position(|v| v.len() >= len).map(|i| b.swap_remove(i))- };- match found {- Some(mut v) => {- v.truncate(len);- v+ // Best fit by capacity: the first fit handed large buffers to small requests (band+ // scratch rows, chroma bands), and once truncated they never served a large one+ // again, so large requests kept allocating fresh (zeroed) memory.+ let mut best: Option<(usize, usize)> = None;+ for (i, v) in b.iter().enumerate() {+ let c = v.capacity();+ if c >= len && best.is_none_or(|(_, bc)| c < bc) {+ best = Some((i, c));+ if c == len {+ break;+ }+ } }- None => vec![fill; len],- }- }-- /// A buffer of `len` copies of `fill` (recycled memory when available).- pub fn take(&self, len: usize, fill: T) -> Vec<T> {- let found = {- let mut b = self.bufs.lock().unwrap_or_else(std::sync::PoisonError::into_inner);- b.iter().position(|v| v.capacity() >= len).map(|i| b.swap_remove(i))+ best.map(|(i, _)| b.swap_remove(i)) }; match found { Some(mut v) => {- v.clear();- v.resize(len, fill);+ // Contents are unspecified: only memory beyond the current length is written.+ if v.len() >= len {+ v.truncate(len);+ } else {+ v.resize(len, fill);+ } v } None => vec![fill; len],@@ -129,12 +129,15 @@ impl<'a> TileDecoder<'a> { let strip_h = sb_rows * 64; let x0 = mi_col_start * 8; let uv_len = (strip_w >> ss_x) * (strip_h >> ss_y);- let planes = [s.pools.samples.take(strip_w * strip_h, 0), s.pools.samples.take(uv_len, 0), s.pools.samples.take(uv_len, 0)];+ // Every sample a later stage reads (inside the mode info grid) is predicted before use, so+ // recycled planes need no clearing.+ let planes = [s.pools.samples.take_any(strip_w * strip_h, 0), s.pools.samples.take_any(uv_len, 0), s.pools.samples.take_any(uv_len, 0)]; let mi_w = mi_col_end - mi_col_start; let strip = Strip { mi_col_start, mi_col_end,- mi: s.pools.mi.take(mi_w * mi_rows, MiInfo::default()),+ // Written for every position of the grid before it is read.+ mi: s.pools.mi.take_any(mi_w * mi_rows, MiInfo::default()), seg_ids: vec![0; mi_w * mi_rows], planes, strides: [strip_w, strip_w >> ss_x, strip_w >> ss_x],