chore(benchmarks): measure builder usage in compilation benchmarks - #406
Merged
Merged
Conversation
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The generated call sites consistently exercise every setter across all builder implementations and preserve an equivalent baseline.
Review effort: Balanced
Findings: None
What changed in this PR
Adds builder usage to compilation benchmarks so setter and build-call costs are measured alongside generated definitions.
Changes:
- Generates complete builder call sites for both benchmark suites and a direct-construction baseline.
- Regenerates benchmark results and compiler diagnostic snapshots with Rust 1.99.
- Updates code generation and lint allowances.
| File | Description |
|---|---|
rust-toolchain.toml |
Updates Rust to 1.99.0. |
bon/tests/integration/ui/compile_fail/diagnostic_on_unimplemented.stderr |
Refreshes compiler diagnostics. |
bon/tests/integration/ui/compile_fail/attr_required.stderr |
Refreshes compiler diagnostics. |
bon/tests/integration/ui/compile_fail/attr_bon.stderr |
Refreshes compiler diagnostics. |
benchmarks/compilation/src/structs_100_fields_10.rs |
Adds generated builder usage for 100 structs. |
benchmarks/compilation/src/structs_10_fields_50.rs |
Adds generated builder usage for 10 structs. |
benchmarks/compilation/src/lib.rs |
Allows the new generated functions under Clippy. |
benchmarks/compilation/results.md |
Records regenerated benchmark measurements. |
benchmarks/compilation/codegen/src/main.rs |
Generates both suites and their builder call sites. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Veetaha
marked this pull request as ready for review
October 2, 2026 20:17
Veetaha
added a commit
that referenced
this pull request
Oct 2, 2026
…king did it. ~30% improvement (#405) I was watching a video from [Theo](https://youtu.be/D8PikZ1KhUo) about his tokenmaxing. While I disagree with him at times, I still like watching his videos to see what's new and interesting in the AI world. One phrase in that video caught my attention. I can't find it right now, but it was smth about "Give Claude your problem, not your solution". I knew that already, but for some reason, the idea of asking Claude to research and optimize `bon` came to me only after he said that, so I gave it a second thought... And Claude did it. In 49 minutes. It found a dumb bottleneck. <sub>...It also suggested a new typestate design with GATs, which looks scary and still doesn't provide a full win over the current design in terms of compile time perf, so I rejected that</sub> The bottleneck was these lines of code: https://github.com/elastio/bon/blob/61fb92455e2715f44f552b23b83a7969e5913636/bon-macros/src/builder/builder_gen/mod.rs#L78-L80 The macro formatted its whole output to a string (just in case it's needed for an error message) and parsed it with `syn` only to add `#[allow]` attributes to each item. That... was about half of the macro's run time. The *KEY WORD* is "macro run time". For some reason I never suspected there could be any bottleneck in bon's macro run time. The pure logic of generating `TokenStream`s and `TokenTree`s, pfft, how hard can it be? Lol, not if I make it harder than it needs to be. IDK why, but I thought formatting the entire resulting `TokenStream` to a string and parsing it into a `syn::File` would cost nothing. Hell no, it's something. It's half of the macro run time spent on doing that... And for what? For adding `#[allow]` attributes to all items in that output, which could be done way more efficiently with a little code change. I am surprised I made that mistake (well, the 2 years younger me did). I would never have thought about this if it wasn't for Claude Opus 5.5 doing all the grunt work of profiling the build times and finding this bottleneck. There are a couple of other less important optimisations, not worth mentioning in this description. So the final result is that `bon` is now faster than `typed-builder` according to the compilation benchmarks in this repo (which I also extended a bit with Claude 🐱 in #406): | Suite | `bon` before | `bon` after | `typed-builder` | `derive_builder` | | :--------------------- | -----------: | ------------: | --------------: | ---------------: | | 100 structs, 10 fields | 2.32 s | 1.56 s (-33%) | 1.66 s | 1.03 s | | 10 structs, 50 fields | 2.10 s | 1.49 s (-29%) | 1.99 s | 0.42 s | Here are the build times of some heavy real-world downstream users of `bon`. The numbers are less impressive here, because a bunch of other things contribute to the build times of these crates. These are clean builds of the crate itself, with its dependencies already compiled: | Crate | Before | After | | :------------------- | -----: | ------------: | | `tellers-mtproto` | 30.2 s | 22.7 s (-25%) | | `frankenstein` | 20.0 s | 17.4 s (-13%) | | `aws_lambda_events` | 13.5 s | 12.0 s (-11%) | | `openai-client-base` | 60.6 s | 55.3 s (-9%) | | `hf-hub` | 3.7 s | 3.3 s (-9%) |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The compilation benchmarks only had the struct definitions, so they didn't measure the cost of calling the builders at all. A typestate change can make the definitions cheaper and the call sites more expensive, and the old benchmarks wouldn't notice that.
This PR adds a builder call with all setters per struct to both suites, and regenerates the results on the latest stable compiler.