Skip to content

perf: PR 61 and PR 62 gains on buffa's proposed packed views - #66

Draft
nyurik wants to merge 1 commit into
mainfrom
buffa-packed-views
Draft

nyurik wants to merge 1 commit into
mainfrom
buffa-packed-views

Conversation

@nyurik

@nyurik nyurik commented Nov 7, 2026

Copy link
Copy Markdown
Owner

An experiment showing what PRs #61 (reader) and #62 (writer) gain together, with the reader built on buffa's proposed packed_views (anthropics/buffa#814) instead of a hand-written wire scanner. buffa comes from the packed-views branch of nyurik/buffa through [patch.crates-io] until that change is released, so this branch is not for merging as is.

What's here

  • Reader, on buffa views: examples/codegen.rs sets packed_views(true), so FeatureView.tags and .geometry are borrowed PackedViews instead of a Vec each. That's the allocation PR 61 removes with its 745-line src/reader/wire.rs; here buffa does it, and wire.rs isn't needed.
  • PR 61's geometry parser: src/geom_reader.rs is PR 61's, unchanged. It parses straight from the packed varints (no temporary vectors, rings sized once) and takes buffa's iterator as is.
  • Layer values resolved once: each layer's values become MvtValueRefs once in MvtReaderRef::new, instead of being matched from the ValueView on every property. PR 61 does the same.
  • PR 62's writer: src/writer.rs is PR 62's, unchanged. It orders values by index, predicts tag keys, boxes and reuses layer state, packs varints in one pass, and writes layers straight into the tile buffer.
  • API: MvtFeatureRef::tags() and geometry_commands() return ExactSizeIterator<Item = u32> instead of slices. That's a breaking change, as PR 61's generate_views(false) also is. Debug output is unchanged.

Results: measured on dedicated AWS hosts over the 207 real-world fixtures, for the reader and writer parts separately before they were combined here. x86 is c8a.large; ARM is c8g.large.

main this branch PR 61 / PR 62 alone
MvtReaderRef::new, x86 instructions 1,221 M 551 M 526 M
  • Decoding alone matches PR 61.
  • Full traversal is 10–12% behind PR 61, which keeps a few offsets per feature where buffa keeps a full view struct.
  • Encoding is PR 62's.
  • Output: every tile encodes byte-identically to v0.7.1, for encode_ref and the streaming builder.

All 338 tests pass, including the 212 fixture snapshots; clippy is clean, and the codegen example reproduces the checked-in generated code. The geometry parser and the writer are @AnthonyKrupko's work from #61 and #62.

An experiment showing what PRs #61 (reader) and #62 (writer) gain together, with the reader built on buffa's proposed `packed_views` (anthropics/buffa#814) instead of a hand-written wire scanner. buffa comes from the `packed-views` branch of nyurik/buffa through `[patch.crates-io]` until that change is released, so this branch is not for merging as is.

**What's here**
- **Reader, on buffa views:** `examples/codegen.rs` sets `packed_views(true)`, so `FeatureView.tags` and `.geometry` are borrowed `PackedView`s instead of a `Vec` each. That's the allocation PR 61 removes with its 745-line `src/reader/wire.rs`; here buffa does it, and `wire.rs` isn't needed.
- **PR 61's geometry parser:** `src/geom_reader.rs` is PR 61's, unchanged. It parses straight from the packed varints (no temporary vectors, rings sized once) and takes buffa's iterator as is.
- **Layer values resolved once:** each layer's values become `MvtValueRef`s once in `MvtReaderRef::new`, instead of being matched from the `ValueView` on every property. PR 61 does the same.
- **PR 62's writer:** `src/writer.rs` is PR 62's, unchanged. It orders values by index, predicts tag keys, boxes and reuses layer state, packs varints in one pass, and writes layers straight into the tile buffer.
- **API:** `MvtFeatureRef::tags()` and `geometry_commands()` return `ExactSizeIterator<Item = u32>` instead of slices. That's a breaking change, as PR 61's `generate_views(false)` also is. `Debug` output is unchanged.

**Results:** measured on dedicated AWS hosts over the 207 real-world fixtures, for the reader and writer parts separately before they were combined here. x86 is c8a.large; ARM is c8g.large.

| | main | this branch | PR 61 / PR 62 alone |
|---|---:|---:|---:|
| `MvtReaderRef::new`, x86 instructions | 1,221 M | 551 M | 526 M |
| full traversal, x86 instructions | 1,925 M | 1,516 M | 1,385 M |
| full traversal, x86 time | 134.6 ms | 95.2 ms | 85.3 ms |
| full traversal, ARM time | 190.9 ms | 129.8 ms | 113.4 ms |
| criterion decode, x86 | 49.6 ms | 37.7 ms | 29.6 ms |
| allocations, full traversal | 1,489,555 | 347,383 | 338,383 |
| reader heap per tile byte | 6.26 B | 5.38 B | 1.82 B |
| `encode_ref`, x86 instructions | 2,551 M | 1,790 M (PR 62) | 1,790 M |
| criterion encode, x86 | 4.27 ms | 3.10 ms (PR 62) | 3.10 ms |

- **Decoding alone** matches PR 61.
- **Full traversal** is 10–12% behind PR 61, which keeps a few offsets per feature where buffa keeps a full view struct.
- **Encoding** is PR 62's.
- **Output:** every tile encodes byte-identically to v0.7.1, for `encode_ref` and the streaming builder.

All 338 tests pass, including the 212 fixture snapshots; clippy is clean, and the codegen example reproduces the checked-in generated code. The geometry parser and the writer are AnthonyKrupko's work from #61 and #62.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@nyurik
nyurik marked this pull request as draft November 7, 2026 23:10
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

Copy/paste detection

ℹ️ Found 3 clones (0 new compared to origin/main) with 22 duplicated lines — 0.37% of 5926 lines in 40 files.

Clones
New Lines Tokens Location Duplicated in
9 61 benches/common/encode.rs:299-307 src/geom_writer.rs:151-159
7 58 src/reader/feature.rs:204-210 src/reader/feature.rs:215-221
6 66 src/writer.rs:672-677 src/writer.rs:685-690

@codecov

codecov Bot commented Nov 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.27%. Comparing base (8ee0411) to head (16bdce3).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #66      +/-   ##
==========================================
+ Coverage   99.20%   99.27%   +0.06%     
==========================================
  Files          11       11              
  Lines        2018     2073      +55     
==========================================
+ Hits         2002     2058      +56     
+ Misses         16       15       -1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

fast-mvt encode/decode — CPU instruction counts

fast-mvt's own encode/decode instruction counts, measured by gungraun under Valgrind/Callgrind — one-shot and deterministic, so these are exact (the mvt/mvt-reader/tinymvt comparison cases run only locally via just bench-cpu, not in CI). A diff vs. the PR base appears once the base branch also carries the gungraun bench. Informational only; this never fails CI.

Results
decoder_cpu::decoder_cpu::decode fast:(setup(Dec :: Fast))
  Baselines:                                              |base
  Instructions:                                 1463611895|1583913792           (-7.59523%) [-1.08220x]
  L1 Hits:                                      2088715997|2284056290           (-8.55234%) [-1.09352x]
  LL Hits:                                          980266|1161100              (-15.5744%) [-1.18447x]
  RAM Hits:                                        7831649|8286330              (-5.48712%) [-1.05806x]
  Total read+write:                             2097527912|2293503720           (-8.54482%) [-1.09343x]
  Estimated Cycles:                             2367725042|2579883340           (-8.22356%) [-1.08960x]

Gungraun result: Ok. 1 without regressions; 0 regressed; 2 filtered; 1 benchmarks finished in 58.6266s

encoder_cpu::encoder_cpu::encode fast:(setup(Enc :: Fast))
  Baselines:                                              |base
  Instructions:                                   73217472|91036978             (-19.5739%) [-1.24338x]
  L1 Hits:                                       107800048|127598805            (-15.5164%) [-1.18366x]
  LL Hits:                                          130070|173575               (-25.0641%) [-1.33447x]
  RAM Hits:                                         311242|315126               (-1.23252%) [-1.01248x]
  Total read+write:                              108241360|128087506            (-15.4942%) [-1.18335x]
  Estimated Cycles:                              119343868|139496090            (-14.4464%) [-1.16886x]
encoder_cpu::encoder_cpu::encode fast_owned:(setup(Enc :: FastOwned))
  Baselines:                                              |base
  Instructions:                                  138959913|156918014            (-11.4443%) [-1.12923x]
  L1 Hits:                                       199693073|219869172            (-9.17641%) [-1.10104x]
  LL Hits:                                          607810|664326               (-8.50727%) [-1.09298x]
  RAM Hits:                                         330329|328304               (+0.61681%) [+1.00617x]
  Total read+write:                              200631212|220861802            (-9.15984%) [-1.10083x]
  Estimated Cycles:                              214293638|234681442            (-8.68744%) [-1.09514x]

Gungraun result: Ok. 2 without regressions; 0 regressed; 4 filtered; 2 benchmarks finished in 7.87292s

@AnthonyKrupko

Copy link
Copy Markdown
Contributor

Thanks for putting this together, iirc PR #62 is not depending on buffa and provides speedups on its own, on #61 I'll take a look into wire.rs run some experiments now to see if we can retain 61 speedup without having to maintain more code

@AnthonyKrupko

AnthonyKrupko commented Nov 8, 2026 •

Copy link
Copy Markdown
Contributor

I think I found one inefficiency in buffa as well, I'll double check it and get it to your buffa pr as a comment, it reduces the gap between this PR and 61/62 by a large margin

@AnthonyKrupko

Copy link
Copy Markdown
Contributor

I profiled this branch against #61 to see where the reader loses time. On reading every feature of the 207 real-world tiles (Callgrind instructions, measured pass only), it was 21% behind #61:

parse only read properties read geometry read everything
#61 527.6 M 866.5 M 994.3 M 1,331.6 M
#66 581.8 M 1,081.2 M 1,116.2 M 1,616.3 M
#66 with nyurik/buffa#1 582.4 M 919.3 M 1,059.3 M 1,398.1 M

Most of the gap was PackedIter::next. It didn't inline, so every tag and geometry element was a call through Buf and buffa's 64-bit decoder. nyurik/buffa#1, against your packed-view-type branch, fixes that in buffa, which takes the gap down to 5%. buffa's own packed benches don't regress. What's left:

  • Parsing, about 55 M: buffa decodes each layer value into a 96-byte ValueView, and grows a Vec of them per layer. MvtReaderRef::new then converts them into a second Vec<MvtValueRef>. perf(reader): read tiles in one pass without per-feature allocations #61 writes one 24-byte MvtValueRef per value into a single Vec for the whole tile. It's also most of why this branch requests about 3× the bytes while parsing (175 MB vs 59 MB over the fixtures).
  • Geometry, about 10 M, after the parse difference.

One small thing on this branch: #61 gave MvtPropertyIter size_hint and ExactSizeIterator, so properties().len() works and properties_vec/to_feature collect into a presized Vec. Since PackedIter already reports its exact length, this branch could keep them at no cost.

#62 doesn't depend on any of this: writer.rs here is #62's, unchanged.

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.

2 participants