bench: compare AnalyticsEvent owned types - #327

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

Thanks for the detailed review. I pushed f69bbef with all four items addressed:

1. Replaced the codename comment with the recursive-size constraint.
2. Moved the comparison into a dedicated analytics_owned_types target/feature; the historical analytics_event build no longer includes the optional codecs or dependencies.
3. Renamed the combined ID to smolstr_smallvec4; all comparison IDs now match the history parser regex.
4. Documented the capacity distribution next to the results: median properties length is 6.5 and only 28% fit inline-4, so the row is explicitly spill-heavy rather than a general SmallVec verdict.

The focused variant test, both isolated bench builds, workspace tests, clippy, rustdoc, fmt, and tracked Markdown lint all pass locally.

[claude code]

Re-reviewed. All three structural fixes landed cleanly, and I verified each by building rather than reading:

• Isolation restored. benches/analytics_event.rs is byte-identical to pre-PR, every variant item is behind the new analytics_owned_types feature, and the history-procedure build produces a binary with zero smol_str/smallvec symbols. Splitting into a separate target and renaming the group to buffa/analytics_owned_types/* is better than what I asked for — the variant rows now can't be confused with the charted series at all.
• ponytail is gone, replaced with the correct infinite-size reason for Nested.children.
• smolstr_smallvec4 now matches parse_criterion.py's id pattern.

I pushed a commit rather than sending it back a second time, because one of the things I found is a hard build break and the rest are small.

The build break. buffa-smolstr is an optional path dependency, and cargo reads a path dep's manifest during resolution even when the feature is off. Dockerfile.bench-buffa copies named directories and never examples/, so task bench-cross and task bench-docker now fail at cargo bench --no-run with failed to load manifest for dependency buffa-smolstr, before any benchmark runs. I reproduced it by replicating the Dockerfile's exact COPY set (exit 101), confirmed one added COPY examples/buffa-smolstr/ line fixes it (exit 0), and pushed that. Nothing in CI would have caught this — no workflow builds benchmarks/ at all.

The isolation guard needed to go on all nine targets, not one. I'd added analytics_owned_types to analytics_event.rs's guard, which was incomplete of me. Enabling the feature pulls the AnalyticsEvent codec plus three variant codecs into the shared lib, so every target linking it loses byte-identical layout. cargo check --bench api_response --features iso,api_response,analytics_owned_types compiled clean before this. It only looked safe because a whole-crate cargo check trips the new target's own guard — and single-target --bench <name> selection, which is exactly what the history procedure and task bench-iso use, never compiles that file. Now guarded in all eight siblings; verified the bad combination errors and all eight normal isolated builds still pass.

On the inline capacity — I got the mechanism wrong the first time, and the corrected version changes the recommendation. I'd written that smallvec4 looks bad because properties (median 6.5, max 10) spills past its 4-element buffer ~72% of the time, and suggested inline-8 as the axis that would show SmallVec winning. Measuring size_of says otherwise:

The 14.9x growth is dominated by sections, whose element type Nested is 376 bytes: its inline buffer alone is 4 * 376 + 16 = 1520, about 79% of the total. So the configuration inlines the wrong field twice over — it pays 1520 bytes to inline a large message type whose lists are short anyway, while under-sizing properties, the field whose 72-byte elements are actually cheap to inline.

That means inline-8 would make things worse, not better — Nested past 600 bytes and the message past 5 KB. The corrected note is now in the bench header, and the real follow-up is to apply SmallList only to small-element fields sized to their distribution, leaving message-typed lists on Vec. I'll file that against #215.

Two things I deliberately left, both worth doing but not at merge time: the nine hand-maintained compile_error! lists are O(n²) and this is the second time a feature had to be added to all of them by hand — a single "exactly one message feature enabled" count assertion in common.rs would make forgetting impossible and delete ~90 lines. And the owned-type variants generate view code no benchmark reads (~1200 lines × 3), which .generate_views(false) would drop.

Nice detail I want to flag as good: the borsh/cfg_aliases entries in the lockfile look alarming but are provably inert — smol_str's optional-dependency closure, never compiled under the enabled feature set.

Merging. Thanks for the quick turnaround on the restructure — the separate target is the right shape, and the parity test over all 50 payloads is what made it safe to be confident about the variants.