Repository navigation
Conversation
Per-thread LZ4 table reuse for 64 B to under 4 KiB stores. Moves the four hand-maintained core pins with the root lock, and documents the per-thread memory the table reuse retains.
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Update cachekit-core version references rust/fuzz/Cargo.toml, tests/unit/protocol/test_envelope_wire_vectors.py, docs/free-threading.md |
The fuzz dependency pin, probe version check, and free-threading audit row now identify cachekit-core 0.7.4. |
Update serialization and performance records docs/features/rust-serialization.md, docs/performance.md, tests/performance/ir_baselines.json |
The serialization documentation describes per-thread LZ4 buffer reuse and clarifies the scope of CACHEKIT_ARROW_COMPRESSION. Selected instruction budgets decrease for serializer benchmarks and increase for byte_storage_store_40b on CPython 3.12 and 3.14. |
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Other
Possibly related issues
- Issue 125 tracks the update of first-party
cachekitpackages to v0.7.4. This pull request updatescachekit-coreand related version references to 0.7.4.
Possibly related PRs
- cachekit-io/cachekit-py#654: Updates the same
cachekit-coreversion references and integration points for the preceding 0.7.2 release. - cachekit-io/cachekit-py#649: Adds the version-drift test that checks consistency across the fuzz pin, documentation, and probe expectation.
- cachekit-io/cachekit-py#636: Updates the same
cachekit-coreintegration points for the preceding 0.7.1 release.
Merge Risk: 
Minimal · up to 2ebc9
-
No actionable merge-blocking risk is established; the reported performance check is non-blocking and documented.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed-
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files. (5 skipped: 5 unsupported.)
-
Full details: Description check
-
Explanation
The description gives detailed change, motivation, testing, performance, and compatibility context. However, it does not follow the repository template. It omits the required Type of Change, Security Checklist, Documentation Validation Checklist, Testing checklist, Backward Compatibility, and Additional Notes sections. It also does not provide the required checkbox confirmations.
Resolution
Reformat the description using all template headings. Select the applicable Type of Change options, complete each required security and dependency checklist item, state the documentation validation outcome, record testing and regression status, confirm backward compatibility, and add any relevant reviewer notes.
-
Finishing Touches
-
Generate docstrings
-
- Commit to this branch
- Create a new PR
Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Providing Context (Files & MCPs)Add these hints in your PR description (or a comment) to unlock deeper checks:
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
Instruction budget: FAILHead
A path fails at +1% or more against the base (+2% for |
The 40 B store rises by about 33 Ir/op: core's new size-class branch and Result return change what fat LTO inlines into the store call. The small-dict serializer round trips, which now reuse the per-thread LZ4 table, fall by 1.2 to 1.7%.
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Providing Context (Files & MCPs)Add these hints in your PR description (or a comment) to unlock deeper checks:
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
|
@kody start-review |
# Conflicts: # docs/performance.md # tests/performance/ir_baselines.json
main re-recorded every budget after an ir_workload.py change, which moves the heap layout the budgets hold at, so the four ByteStorage paths this bump moves are re-measured on the merged tree with --update --allow-increase on CPython 3.12.12 and 3.14.3.
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Providing Context (Files & MCPs)Add these hints in your PR description (or a comment) to unlock deeper checks:
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
main's Rust decode lowered serializer_default by 17% and serializer_encrypted by 10% without moving their budgets; both run through ByteStorage, so they are lowered to what this tree measures on CPython 3.12.12 and 3.14.3.
5c71d52
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Providing Context (Files & MCPs)Add these hints in your PR description (or a comment) to unlock deeper checks:
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
|
@kody start-review |
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Providing Context (Files & MCPs)Add these hints in your PR description (or a comment) to unlock deeper checks:
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
# Conflicts: # docs/performance.md # tests/performance/ir_baselines.json
….7.4 main now runs this path over 300,000 calls, which holds its figure within a few instructions, so both budgets come from the gate's own --update --allow-increase on CPython 3.12.12 and 3.14.3.
|
@kody start-review |
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Providing Context (Files & MCPs)Add these hints in your PR description (or a comment) to unlock deeper checks:
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
Bumps cachekit-core from 0.7.2 to 0.7.4. 0.7.4 reuses one LZ4 hash table and one output scratch per thread for ByteStorage stores from 64 B to under 4 KiB (cachekit-io/cachekit-core#144); compressed bytes are unchanged. 0.7.3 moved lz4_flex to 0.13 (cachekit-io/cachekit-core#141).
What moved
Cargo.lockcachekit-core0.7.2 → 0.7.4,lz4_flex0.12.2 → 0.13.1 (cargo update -p cachekit-core --precise 0.7.4; nothing else moved)rust/fuzz/Cargo.toml,rust/fuzz/Cargo.lock=0.7.2→=0.7.4, same two lock entriestests/unit/protocol/test_envelope_wire_vectors.pyPROBE_VERIFIED_CORE_VERSION→0.7.4docs/free-threading.mddocs/features/rust-serialization.mdtests/performance/ir_baselines.json,docs/performance.mdrust/Cargo.tomlkeepsversion = "0.7.0": the caret already admits 0.7.4.Probe verification
0.7.4's Release run 41153609911 (push to
mainat3967998, thecachekit-core-v0.7.4tag commit) rancargo test --locked --all-featuresin itsverifyjob, and the log shows both allocation-probe tests passing:Tests
tests/unit/protocolandtests/unit/test_core_version_drift.py: 1005 passed, against an extension rebuilt from this lock.Full non-slow suite (
--ignore=tests/fuzzing): 6858 passed. Every error and 16 of the 17 failures areRedisMisconfigured(noredis-serveron the test host); the 17th,test_eviction_handles_stat_failure, is a pure-PythonFileBackendtest that fails under one basetemp and passes under another, independent of the extension.ruff check,ruff format --check, basedpyright: pass.cargo metadata --lockedpasses on both lockfiles.Instruction counts (
perf-irCI, 3.12, head against base on one runner):serializer_default-1.36% andserializer_auto-1.18%, from the table reuse.byte_storage_store_40bis +1.27% (2,755 to 2,790 Ir/op) and fails the non-required 1% check. On inputs under 64 B, 0.7.4'scompress_blockadds one size-class compare and returns aResultthat the caller unwraps; that path makes no LZ4 call and touches no thread local. Every other path is within 1%.Where the 40 B cost goes: a callgrind loop of 40 B
store()calls on the shipped (fat-LTO) build, rebuilt with symbols, puts the whole difference in the store call: +24 Ir/op in the PyO3storetrampoline thatByteStorage::storeinlines into, +6 Ir/op fromrmp::marker::Marker::to_u8no longer being inlined, and +3 Ir/op incompress_block(2,617.5 against 2,584.5 Ir/op in total, the same on two builds). The new branch moved inlining; no new work runs.Budgets re-recorded on the tree merged with
main(docs/performance.md, Instruction budgets: re-record after an extension change, and after anyir_workload.pychange). They cover only the four paths that run through ByteStorage. Recorded on CPython 3.12.12 and 3.14.3, x86_64, glibc 2.39. On that host,main's own tree reproducesmain's budgets for these paths within 0.31%.byte_storage_store_40b(raised,--update --allow-increase, 300,000 calls)serializer_defaultserializer_autoserializer_encryptedMost of the
serializer_defaultandserializer_encrypteddrop (about 17% and 10%) comes frommain's Rust decode, which landed without moving these budgets. This tree measures it, and both paths run through ByteStorage, so they are ratcheted here.40 B runs on the tree merged with
main, which now runs this path over 300,000 calls: on 3.12, 2,690, 2,692, 2,692, 2,692 and 2,694; on 3.14, 2,565 in all five runs. Both budgets are the gate's own--updatefigure; none is set by hand. Confirmation runs at these budgets put all four paths atokorWARN(under 1%) on both interpreters.The CI instruction check compares head against base, not against these budgets, so it stays red on the 40 B path: about +1.3% to +1.5% at 300,000 calls (base 2,656 to 2,658, head 2,690 to 2,696 on 3.12). The budgets keep a local
make perf-ironmainpassing after merge.Closes LAB-12129