compiler: several optimizations and cleanups - #1034
Open
apoelstra wants to merge 10 commits into
Open
Conversation
It feels a little weird to export this at the root but there isn't anywhere else in the public API where it really fits, so just dump it there.
apoelstra
force-pushed
the
2026-08/compiler-opts-1
branch
from
August 23, 2026 20:44
2be0c48 to
03b9b1e
Compare
If we want to regression test the Concrete parser, that's fine but belongs in a separate fuzztest (which I may add in a separate PR). But for testing the compiler, we definitely want to be synthesizing policies directly rather than parsing and rejecting.
Gets rid of a couple 'as f64' casts.
These policies exercise paths that earlier iterations of this PR introduced bugs to. (The bugs were found by fuzzing.)
Better encapsulation, and I want to clean up these types. Code move only.
The Type::type_check function takes a terminal, matches on it, calls the appropriate method on Type, and wraps it in a nice error. It turns out that in the compiler, this is obscenely slow. I'm not sure if it's because the big match undermines inlining, or if the error-wrapping (which the compiler doesn't even use; it only uses typeck errors as a binary "ok or not?" signal), or what. But empirically, replacing the call to Type::type_check with direct calls to the methods on Type, results in a massive compiler speedup. At the cost of a bit more code repetition. IMO definitely worth it. You shouldn't trust these benchmarks too much because my system was busy, but the difference is quite extreme: Before: test benchmarks::compiler_benches::compile_basic ... bench: 6,285,527.70 ns/iter (+/- 4,895,149.56) test benchmarks::compiler_benches::compile_large ... bench: 14,602,446.20 ns/iter (+/- 9,250,348.77) test benchmarks::compiler_benches::compile_large_tap ... bench: 982,121,582.90 ns/iter (+/- 1,209,655,507.47) test benchmarks::compiler_benches::compile_xlarge ... bench: 582,951,636.20 ns/iter (+/- 58,752,609.07) After: test benchmarks::compiler_benches::compile_basic ... bench: 1,520,658.20 ns/iter (+/- 13,249.16) test benchmarks::compiler_benches::compile_large ... bench: 4,128,737.40 ns/iter (+/- 55,408.00) test benchmarks::compiler_benches::compile_large_tap ... bench: 716,569,601.10 ns/iter (+/- 130,612,914.15) test benchmarks::compiler_benches::compile_xlarge ... bench: 148,633,378.80 ns/iter (+/- 16,348,954.09) On `compile_large_tap` it's "only" a 35% speedup but on the others, and on the `segwit_limits` unit tests, it's a 3-4x speedup. Wild.
This is a weird structure. I'm not sure what I was thinking here. It's 4 function pointers in a struct, with a `cast()` method on them that calls all the functions in the right order to transform an AstElemExt. Why not just make it an AstElemExt method directly? I did this to simplify the code, and it also comes with another fairly big speedup (though this one is small enough that it's plausibly just noise from my busy machine). The weird order is copied from the old logic. I don't think there's any rhyme or reason to it, but if we change the order then some compilations can change (e.g. swapping the l and n wrappers, which functionally commute). Before (copied from the "after" from my previous commit) test benchmarks::compiler_benches::compile_basic ... bench: 1,520,658.20 ns/iter (+/- 13,249.16) test benchmarks::compiler_benches::compile_large ... bench: 4,128,737.40 ns/iter (+/- 55,408.00) test benchmarks::compiler_benches::compile_large_tap ... bench: 716,569,601.10 ns/iter (+/- 130,612,914.15) test benchmarks::compiler_benches::compile_xlarge ... bench: 148,633,378.80 ns/iter (+/- 16,348,954.09) After: test benchmarks::compiler_benches::compile_basic ... bench: 1,199,013.60 ns/iter (+/- 28,096.21) test benchmarks::compiler_benches::compile_large ... bench: 3,350,305.90 ns/iter (+/- 463,149.87) test benchmarks::compiler_benches::compile_large_tap ... bench: 703,996,003.60 ns/iter (+/- 835,362,175.58) test benchmarks::compiler_benches::compile_xlarge ... bench: 140,988,430.90 ns/iter (+/- 24,692,231.52) seems like 30% or so on the small ones, 5% or so on the big ones. Nice.
This simplifies and cleans up the code, but with a pretty severe performance hit for the "small" benchmarks, in exchange for a improvement on the larger ones. Will keep an eye on this. Before: test benchmarks::compiler_benches::compile_basic ... bench: 1,199,013.60 ns/iter (+/- 28,096.21) test benchmarks::compiler_benches::compile_large ... bench: 3,350,305.90 ns/iter (+/- 463,149.87) test benchmarks::compiler_benches::compile_large_tap ... bench: 703,996,003.60 ns/iter (+/- 835,362,175.58) test benchmarks::compiler_benches::compile_xlarge ... bench: 140,988,430.90 ns/iter (+/- 24,692,231.52) After: test benchmarks::compiler_benches::compile_basic ... bench: 1,570,666.35 ns/iter (+/- 1,066,467.89) test benchmarks::compiler_benches::compile_large ... bench: 6,010,996.75 ns/iter (+/- 3,744,165.23) test benchmarks::compiler_benches::compile_large_tap ... bench: 694,140,897.60 ns/iter (+/- 656,723,194.35) test benchmarks::compiler_benches::compile_xlarge ... bench: 135,546,606.00 ns/iter (+/- 11,686,439.12)
There is no longer any reason to keep this as a separate data structure.
apoelstra
force-pushed
the
2026-08/compiler-opts-1
branch
from
August 24, 2026 01:52
03b9b1e to
d4a97c6
Compare
Member
Author
|
On d4a97c6 successfully ran local tests |
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
1 participant
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.
This is a followup to #988 and starts a project to optimize the compiler. It pulls apart the compiler module into multiple submodules, inlines the
CompilerExtDatastruct (which since #988 is just a pair of floats which never change after construction) (previously it also had abranch_probvalue that would be mutated during compilation).This also cleans up the cast-computation logic, type checking within the compiler, and eliminates a few other places where we were passing miscellaneous closures around. It leaves the codebase in a much cleaner state.
Some of the commits have benchmark data showing a significant speedup, but I don't have a spare computer right now to run clean before and after benchmarks on the whole branch. Will update the PR with that data once I have it.
It also greatly speeds up the existing regression test between the compiler on master and the one released on 12.x. I ran 150 MM iterations just now, plus a whole bunch when I was developing the branch some months ago.