perf(claims): intern common claim/header keys in json_to_bound (#123) - #129
Merged
Merged
Conversation
json_to_bound allocated a fresh PyString for every dict key on every
decode, including the same standard claim/header names every time:
exp, iat, nbf, sub, aud, iss, jti, alg, typ, kid.
Those ten keys now come from pyo3::intern!, which caches each key in
a call-site-local static rather than allocating it fresh. Interning
also registers the string in CPython's own intern table, so a
subsequent payload.get("exp") on the Python side (a str literal,
which CPython also interns) can hit the identity-comparison fast path
during dict lookup instead of a full string comparison. Any other key
still falls back to an ordinary, uninterned PyString, unchanged from
before.
Measured (native decode, 8-claim payload, release build):
~2.05us -> ~1.92us (~6% less).
No behavioural change.
Closes #123
Regression coverage for the previous commit: each standard claim name and each standard header field must decode back as the exact same interned str object pyo3 caches for it (identity, not just equality), while a key outside that list must still decode correctly as an ordinary, uninterned str.
3 tasks
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.
Problem
json_to_bound(used by everydecode/decode_completecall) allocated a freshPyStringfor every dict key on every call, including the same standard claim/header names every time.Fix
The ten standard names —
exp,iat,nbf,sub,aud,iss,jti,alg,typ,kid— now come frompyo3::intern!, which caches each key in a call-site-local static instead of allocating it fresh. Interning also registers the string in CPython's own intern table, so a laterpayload.get("exp")on the Python side (astrliteral, which CPython auto-interns) can hit the identity-comparison fast path during dict lookup instead of a full string comparison. Any other key still falls back to an ordinary, uninternedPyString, unchanged from before.No behavioral change.
Results
Native
decode, 8-claim payload (sub,iat,exp,nbf,iss,jti, plus two custom keys), release build, averaged over repeated runs:decodeModest, as expected — allocation is a small fraction of total decode cost next to base64/JSON/signature verification — but real and reproducible.
Tests
Added
test_standard_claim_key_is_interned,test_standard_header_key_is_internedandtest_custom_claim_key_is_not_interned(tests/test_encode_decode.py): each standard name must decode back as the exact same interned object (is, not just==), while a key outside the list must still decode correctly as an ordinary, uninterned string.(Considered a
#[cfg(test)]Rust unit test instead/also, but this crate's existing Rust tests are pure-logic and never touch the Python API — adding one that does would need theauto-initializepyo3 feature as a new dev-dependency, which changescargo test's requirements project-wide for a change this contained. The Python-level identity test gives the same guarantee through the existing test infrastructure.)Checklist
cargo build/clippy --all-targets -D warnings/fmt --check/cargo testclean with bothaws_lc_rs(default) and--no-default-features --features rust_cryptopytest(270 passed, 1 skipped) andmypycleanCloses #123.