perf: return static Poseidon instances instead of cloning per hash call - #58
Open
wstran wants to merge 1 commit into
Open
perf: return static Poseidon instances instead of cloning per hash call#58wstran wants to merge 1 commit into
wstran wants to merge 1 commit into
Conversation
Every scalar tweakable-hash call (chain steps in sign and verify, Merkle path nodes, message hashing) fetches its Poseidon permutation through poseidon1_16()/poseidon1_24(), which do get_or_init(...).clone() and so pay a heap-allocating clone of the round-constant tables on each hash. Root cause: the OnceLock getters in lib.rs return owned instances; the Vec-backed layer tables make that clone ~450ns, on par with the ~900ns width-16 permutation itself. The SIMD key-generation path is unaffected (it hoists the instance out of the loop), which is why only sign, verify and path verification are slowed. Fix: return &'static references from both getters and pass them through at the call sites. No arithmetic changes. Measured (M3 Pro, one chain step = tweak encode + instance fetch + width-16 permutation): chain step: 1318 ns -> 908 ns (clone alone: 452 ns) Criterion, lifetime 2^18 (change in mean, all p < 0.05): verify w1: -13.4% w2: -24.0% w4: -29.9% w8: -31.0% sign w1: -7.5% w2: -12.8% w4: -19.0% w8: -18.4% (10% offset variants within 2 points of the above) Signatures are byte-identical to main: a seeded 32-signature digest matches across both builds, and all 111 tests pass.
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.
What
Every scalar tweakable-hash call fetches its Poseidon permutation through
poseidon1_16()/poseidon1_24(), and those getters doget_or_init(...).clone(): each hash pays a heap-allocating clone of the round-constant tables. That cost lands on every chain step in sign and verify, every Merkle path node, and every message hash. The SIMD key-generation path is unaffected because it hoists the instance out of its loops, which is why only the scalar paths are slowed.Root cause: the
OnceLockgetters inlib.rsreturn owned instances; theVec-backed layer tables make that clone ~450ns, on par with the ~900ns width-16 permutation itself.Fix: return
&'staticreferences from both getters and pass them through at the call sites. No arithmetic changes; net diff is -4 lines.This also answers part of #27 (hotspot profiling): the per-step anomaly was found by regressing verify time against the deterministic chain-step count (
v * (w-1) - target_sum), which showed each step costing ~1.3us against a ~0.9us raw permutation.Numbers
Apple M3 Pro (aarch64 NEON). One chain step (tweak encode + instance fetch + width-16 permutation):
Criterion, lifetime 2^18, change in mean (all p < 0.05; 10% offset variants within 2 points):
repro:
Correctness
Sync; the SIMD paths already share one instance per call today.