You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
CacheKit::__interop_read, the read a #[cachekit] function re-polls while another worker fills a cold key, now awaits one read_bytes for plain and encrypting clients and opens an encrypting client's bytes synchronously afterwards with SecureCache::open. SecureCache::__interop_read, which repeated the namespace reject, key resolution and read, is deleted. This is the shape #228 gave interop_read_swr.
Why
An encrypting client's read used to return secure.__interop_read(key).await before its own read, so the secure read's future sat in the state of __interop_read's future. The #[cachekit] poll function that awaits it is generic and instantiated in the calling crate, and plain and secure reads share it. #228 measured what that coupling costs on the SWR read: a change confined to the secure read added 34 Ir/op to macro_hit/plain. __interop_read carried the same coupling, plus a second copy of the reject, resolve and read.
The order of operations is unchanged for both clients: reject a namespaced interop client, resolve the key, read L1 then the backend, then decode (plain) or decrypt and decode (encrypting). No AAD, key, key-format or wire change.
Instruction counts
make bench-instr BASE=<main e3a0123 build>, callgrind, release build with --features macros: 122 of 122 cases within noise, no SLOWER case.
Case
40 B
64 B
1 KiB
64 KiB
macro_hit/plain
5,512 → 5,515
5,559 → 5,562
6,088 → 6,091
99,947 → 99,950
macro_hit/rel
5,512 → 5,515
5,559 → 5,562
6,088 → 6,091
99,963 → 99,966
macro_hit/enc
9,672 → 9,675
9,786 → 9,789
13,130 → 13,133
336,715 → 336,718
Every macro_hit cell moves +3 Ir/op (at most +0.05%, against a 0.50% floor). macro_hit measures a hit on the first read, __interop_read_swr, which no code here changes; the poll function it runs in also holds the __interop_read re-poll, so the +3 is that function's layout.
Tests
make quick-check passes (fmt, clippy -D warnings, the native test suite).
decode_bounds_tests still refuses hostile sealed plaintext on CacheKit::__interop_read from the backend, from L1 and from L1 again, and decodes every sealed accept vector on each of those reads.
Mutation check: disabling the new encrypting branch, so an encrypting client decodes ciphertext as plaintext, fails every_sealed_accept_vector_decodes_from_the_backend_and_from_l1 on CacheKit::__interop_read. The test reaches the moved open call.
cargo clippy --no-default-features --features cachekitio,l1,macros -D warnings (no encryption) and the wasm32 workers,cachekitio,encryption build compile.
Docs: none needed. Both functions are hidden macro plumbing or private; only their rustdoc changed.
…text synchronously
An encrypting client's CacheKit::__interop_read awaited
SecureCache::__interop_read before its own read, which repeated the
reject, key resolution and read_bytes. That kept a secure future in the
state of the generic #[cachekit] poll function every plain read shares,
the coupling interop_read_swr already dropped.
Both clients now reject, resolve and await read_bytes once; an
encrypting client then calls SecureCache::open synchronously on the
bytes. SecureCache::__interop_read is deleted. No AAD, key, key-format
or wire change.
No actionable comments were generated in the recent review.
Summary by CodeRabbit
Bug Fixes
Interoperability reads now work with both encrypted and unencrypted cached data. Encrypted data is decrypted before it is decoded, while unencrypted data is decoded directly. This allows cached data to be read through the appropriate path based on the cache’s encryption settings. The change applies to interoperability reads and does not alter other cache operations.
Walkthrough
The interop read method now performs a shared read before selecting encrypted or plain decoding. The change removes the separate encrypted interop read helper.
Changes
Interop cache reads
Layer / File(s)
Summary
Shared interop read and decoding crates/cachekit/src/client.rs
__interop_read rejects namespaced clients, resolves the key and reads the value. On a hit, it uses SecureCache::open with the interop decoder when encryption is configured; otherwise, it decodes the payload directly. The separate SecureCache::__interop_read helper was removed. A comment now describes branching on the encryption handle before encoding or decoding.
The change shares the interop read before decoding; no material merge risk is identified.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title describes the shared read and synchronous ciphertext opening in __interop_read.
Description check
The description matches the shared read, synchronous opening, and removal of the secure read helper.
Docstring Coverage
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. 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 1 functions across 1 files.
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.
Reviewing files that changed from the base of the PR and between c97dad0 and b4a64d5.
Files selected for processing: 1
crates/cachekit/src/client.rs
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
122 of 122 cases measured. A case fails when its median is slower than the base by its floor or more (SLOWER). The floor is 0.5%, and such a case also fails when the head's runs spread by more than that (NOISIER), except l2_hit, whose floor is 2% because its count varies between runs. delete cases are reported, never failed. Method: README, Measuring performance.
122 cases within noise
Verdict
Case
Base Ir/op
Head Ir/op
Delta
Floor
Head spread
within noise
l1_hit/plain/40
1284
1285
+0.08%
0.50%
0.00%
within noise
l1_hit/plain/64
1331
1332
+0.08%
0.50%
0.00%
within noise
l1_hit/plain/1024
1860
1861
+0.05%
0.50%
0.00%
within noise
l1_hit/plain/65536
95691
95692
+0.00%
0.50%
0.00%
within noise
l1_hit/rel/40
1284
1285
+0.08%
0.50%
0.00%
within noise
l1_hit/rel/64
1331
1332
+0.08%
0.50%
0.00%
within noise
l1_hit/rel/1024
1860
1861
+0.05%
0.50%
0.00%
within noise
l1_hit/rel/65536
95707
95708
+0.00%
0.50%
0.00%
within noise
l1_hit/enc/40
4689
4690
+0.02%
0.50%
0.00%
within noise
l1_hit/enc/64
4803
4804
+0.02%
0.50%
0.00%
within noise
l1_hit/enc/1024
8145
8146
+0.01%
0.50%
0.00%
within noise
l1_hit/enc/65536
331659
331660
+0.00%
0.50%
0.00%
within noise
l1_hit/prev/40
7587
7588
+0.01%
0.50%
0.00%
within noise
l1_hit/prev/64
7775
7776
+0.01%
0.50%
0.00%
within noise
l1_hit/prev/1024
14013
14014
+0.01%
0.50%
0.00%
within noise
l1_hit/prev/65536
632682
632683
+0.00%
0.50%
0.00%
within noise
l1_hit/ns/40
1485
1486
+0.07%
0.50%
0.00%
within noise
l1_hit/ns/64
1529
1530
+0.07%
0.50%
0.00%
within noise
l1_hit/ns/1024
2064
2065
+0.05%
0.50%
0.00%
within noise
l1_hit/ns/65536
95889
95890
+0.00%
0.50%
0.00%
within noise
l1_hit/nsenc/40
4891
4892
+0.02%
0.50%
0.00%
within noise
l1_hit/nsenc/64
5005
5006
+0.02%
0.50%
0.00%
within noise
l1_hit/nsenc/1024
8359
8360
+0.01%
0.50%
0.00%
within noise
l1_hit/nsenc/65536
331893
331894
+0.00%
0.50%
0.00%
within noise
l1_hit_row/plain/40
7777
7771
-0.08%
0.50%
0.00%
within noise
l1_hit_row/plain/64
7824
7818
-0.08%
0.50%
0.00%
within noise
l1_hit_row/plain/1024
8339
8333
-0.07%
0.50%
0.00%
within noise
l1_hit_row/plain/65536
102324
102318
-0.01%
0.50%
0.00%
within noise
l1_hit_row/enc/40
11921
11920
-0.01%
0.50%
0.00%
within noise
l1_hit_row/enc/64
11830
11829
-0.01%
0.50%
0.00%
within noise
l1_hit_row/enc/1024
15024
15023
-0.01%
0.50%
0.00%
within noise
l1_hit_row/enc/65536
339117
339116
-0.00%
0.50%
0.00%
within noise
l2_hit/plain/40
6344
6368
+0.38%
2.00%
1.44%
within noise
l2_hit/plain/64
6375
6351
-0.38%
2.00%
1.05%
within noise
l2_hit/plain/1024
7171
7179
+0.10%
2.00%
0.16%
within noise
l2_hit/plain/65536
166681
166623
-0.03%
2.00%
0.09%
within noise
l2_hit/rel/40
7006
7003
-0.04%
2.00%
0.34%
within noise
l2_hit/rel/64
7022
7028
+0.09%
2.00%
0.73%
within noise
l2_hit/rel/1024
7821
7823
+0.02%
2.00%
0.79%
within noise
l2_hit/rel/65536
167214
167221
+0.00%
2.00%
0.12%
within noise
l2_hit/enc/40
9757
9770
+0.12%
2.00%
0.41%
within noise
l2_hit/enc/64
9892
9852
-0.41%
2.00%
0.75%
within noise
l2_hit/enc/1024
13487
13499
+0.08%
2.00%
0.34%
within noise
l2_hit/enc/65536
402807
402661
-0.04%
2.00%
0.05%
within noise
l2_hit/prev/40
12671
12647
-0.18%
2.00%
0.21%
within noise
l2_hit/prev/64
12823
12842
+0.15%
2.00%
0.41%
within noise
l2_hit/prev/1024
19386
19376
-0.05%
2.00%
0.32%
within noise
l2_hit/prev/65536
703801
703730
-0.01%
2.00%
0.02%
within noise
l2_hit/ns/40
6778
6817
+0.58%
2.00%
0.42%
within noise
l2_hit/ns/64
6972
6967
-0.07%
2.00%
0.57%
within noise
l2_hit/ns/1024
7596
7590
-0.09%
2.00%
0.13%
within noise
l2_hit/ns/65536
167140
167103
-0.02%
2.00%
0.13%
within noise
l2_hit/nsenc/40
10443
10429
-0.14%
2.00%
0.42%
within noise
l2_hit/nsenc/64
10372
10384
+0.12%
2.00%
0.53%
within noise
l2_hit/nsenc/1024
13913
13913
+0.00%
2.00%
0.29%
within noise
l2_hit/nsenc/65536
403262
403237
-0.01%
2.00%
0.06%
within noise
set/plain/40
4507
4510
+0.07%
0.50%
0.00%
within noise
set/plain/64
4514
4517
+0.07%
0.50%
0.00%
within noise
set/plain/1024
4658
4661
+0.06%
0.50%
0.00%
within noise
set/plain/65536
70083
70086
+0.00%
0.50%
0.00%
within noise
set/rel/40
5293
5296
+0.06%
0.50%
0.00%
within noise
set/rel/64
5327
5330
+0.06%
0.50%
0.00%
within noise
set/rel/1024
5447
5450
+0.05%
0.50%
0.00%
within noise
set/rel/65536
70877
70880
+0.00%
0.50%
0.00%
within noise
set/enc/40
7655
7658
+0.04%
0.50%
0.00%
within noise
set/enc/64
7720
7723
+0.04%
0.50%
0.00%
within noise
set/enc/1024
10783
10786
+0.03%
0.50%
0.00%
within noise
set/enc/65536
305929
305932
+0.00%
0.50%
0.00%
within noise
set/ns/40
4719
4721
+0.06%
0.50%
0.00%
within noise
set/ns/64
4736
4739
+0.06%
0.50%
0.00%
within noise
set/ns/1024
4878
4881
+0.06%
0.50%
0.00%
within noise
set/ns/65536
70285
70288
+0.00%
0.50%
0.00%
within noise
set/nsenc/40
7845
7848
+0.04%
0.50%
0.00%
within noise
set/nsenc/64
7947
7950
+0.04%
0.50%
0.00%
within noise
set/nsenc/1024
11004
11007
+0.03%
0.50%
0.00%
within noise
set/nsenc/65536
306091
306094
+0.00%
0.50%
0.00%
within noise
delete/plain/40
3742
3765
+0.60%
2.00%
1.13%
within noise
delete/plain/64
3754
3771
+0.44%
2.00%
0.69%
within noise
delete/plain/1024
3750
3733
-0.47%
3.87%
1.31%
within noise
delete/plain/65536
3974
3966
-0.20%
2.00%
1.27%
within noise
delete/rel/40
4394
4382
-0.26%
2.88%
1.04%
within noise
delete/rel/64
4394
4402
+0.18%
4.67%
1.16%
within noise
delete/rel/1024
4388
4388
-0.01%
6.59%
0.96%
within noise
delete/rel/65536
4595
4601
+0.13%
2.00%
0.72%
within noise
delete/enc/40
3920
3922
+0.07%
4.13%
1.68%
within noise
delete/enc/64
3715
3726
+0.31%
3.36%
1.20%
within noise
delete/enc/1024
3762
3709
-1.43%
3.84%
1.59%
within noise
delete/enc/65536
3964
3961
-0.07%
2.14%
0.83%
within noise
delete/parked256/64
3821
3827
+0.14%
2.00%
0.11%
within noise
delete/parked1024/64
3829
3829
+0.00%
2.00%
0.05%
within noise
exists/plain/40
722
723
+0.14%
0.50%
0.00%
within noise
exists/plain/64
722
723
+0.14%
0.50%
0.00%
within noise
exists/plain/1024
722
723
+0.14%
0.50%
0.00%
within noise
exists/plain/65536
728
729
+0.14%
0.50%
0.00%
within noise
exists/rel/40
722
723
+0.14%
0.50%
0.00%
within noise
exists/rel/64
722
723
+0.14%
0.50%
0.00%
within noise
exists/rel/1024
722
723
+0.14%
0.50%
0.00%
within noise
exists/rel/65536
728
729
+0.14%
0.50%
0.00%
within noise
exists/enc/40
740
743
+0.41%
0.50%
0.00%
within noise
exists/enc/64
740
743
+0.41%
0.50%
0.00%
within noise
exists/enc/1024
740
743
+0.41%
0.50%
0.00%
within noise
exists/enc/65536
746
749
+0.40%
0.50%
0.00%
within noise
oneshot/enc/40
69448
69447
-0.00%
0.50%
0.00%
within noise
oneshot/enc/64
69562
69561
-0.00%
0.50%
0.00%
within noise
oneshot/enc/1024
73224
73223
-0.00%
0.50%
0.00%
within noise
oneshot/enc/65536
462632
462631
-0.00%
0.50%
0.00%
within noise
oneshot/prev/40
101773
101772
-0.00%
0.50%
0.00%
within noise
oneshot/prev/64
101961
101960
-0.00%
0.50%
0.00%
within noise
oneshot/prev/1024
108513
108512
-0.00%
0.50%
0.00%
within noise
oneshot/prev/65536
793064
793063
-0.00%
0.50%
0.00%
within noise
macro_hit/plain/40
5203
5200
-0.06%
0.50%
0.00%
within noise
macro_hit/plain/64
5250
5247
-0.06%
0.50%
0.00%
within noise
macro_hit/plain/1024
5779
5776
-0.05%
0.50%
0.00%
within noise
macro_hit/plain/65536
99642
99639
-0.00%
0.50%
0.00%
within noise
macro_hit/rel/40
5203
5200
-0.06%
0.50%
0.00%
within noise
macro_hit/rel/64
5250
5247
-0.06%
0.50%
0.00%
within noise
macro_hit/rel/1024
5779
5776
-0.05%
0.50%
0.00%
within noise
macro_hit/rel/65536
99658
99655
-0.00%
0.50%
0.00%
within noise
macro_hit/enc/40
9224
9218
-0.07%
0.50%
0.00%
within noise
macro_hit/enc/64
9338
9332
-0.06%
0.50%
0.00%
within noise
macro_hit/enc/1024
12690
12684
-0.05%
0.50%
0.00%
within noise
macro_hit/enc/65536
336226
336220
-0.00%
0.50%
0.00%
PR head b4a64d547df5691feaeb6b952f9bb5f48e3fbf42 merged into its base, measured against e3a01230d64145f19e14b4e314c7631bf05e2e7a on the same runner. Workflow run
…e the interop reads follow
Both interop reads now resolve and read before they branch on encrypting(), so 'every value method routes through it first' no longer held. What holds, and what the protocol rule needs, is that every value method branches on it before it encodes or decodes a value.
Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.
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
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.
![kodus-27b[bot]](https://avatars.githubusercontent.com/u/223823690?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
CacheKit::__interop_read, the read a#[cachekit]function re-polls while another worker fills a cold key, now awaits oneread_bytesfor plain and encrypting clients and opens an encrypting client's bytes synchronously afterwards withSecureCache::open.SecureCache::__interop_read, which repeated the namespace reject, key resolution and read, is deleted. This is the shape #228 gaveinterop_read_swr.Why
An encrypting client's read used to
return secure.__interop_read(key).awaitbefore its own read, so the secure read's future sat in the state of__interop_read's future. The#[cachekit]poll function that awaits it is generic and instantiated in the calling crate, and plain and secure reads share it. #228 measured what that coupling costs on the SWR read: a change confined to the secure read added 34 Ir/op tomacro_hit/plain.__interop_readcarried the same coupling, plus a second copy of the reject, resolve and read.The order of operations is unchanged for both clients: reject a namespaced interop client, resolve the key, read L1 then the backend, then decode (plain) or decrypt and decode (encrypting). No AAD, key, key-format or wire change.
Instruction counts
make bench-instr BASE=<main e3a0123 build>, callgrind, release build with--features macros: 122 of 122 cases within noise, no SLOWER case.macro_hit/plainmacro_hit/relmacro_hit/encEvery
macro_hitcell moves +3 Ir/op (at most +0.05%, against a 0.50% floor).macro_hitmeasures a hit on the first read,__interop_read_swr, which no code here changes; the poll function it runs in also holds the__interop_readre-poll, so the +3 is that function's layout.Tests
make quick-checkpasses (fmt, clippy-D warnings, the native test suite).decode_bounds_testsstill refuses hostile sealed plaintext onCacheKit::__interop_readfrom the backend, from L1 and from L1 again, and decodes every sealed accept vector on each of those reads.every_sealed_accept_vector_decodes_from_the_backend_and_from_l1onCacheKit::__interop_read. The test reaches the movedopencall.cargo clippy --no-default-features --features cachekitio,l1,macros -D warnings(no encryption) and the wasm32workers,cachekitio,encryptionbuild compile.Docs: none needed. Both functions are hidden macro plumbing or private; only their rustdoc changed.