test: add comprehensive node cache correctness tests for BTreeMap#431
Open
test: add comprehensive node cache correctness tests for BTreeMap#431
Conversation
Run the same randomized operation sequence at cache sizes 0, 1, 4, 16, 64, 256 to catch size-dependent correctness bugs. Each variant uses 3 proptest cases to keep total runtime reasonable.
Run a deterministic operation sequence (insert, overwrite, get, remove, pop, iterate) with cache=0 and cache=1/4/16/64/256, assert all return values are identical. This is the strongest cache invariant: the cache must be invisible to the API.
Add 8 deterministic tests that exercise specific cache-interaction patterns with small caches (0, 1, 4 slots) to maximize collision and eviction pressure: - insert-then-get - overwrite-then-get (the pattern that broke save_and_cache_node) - split-then-get-all - merge-then-get-remaining - sequential-inserts-then-gets - interleaved-insert-get-remove - pop correctness with cache - first/last_key_value during mutations Each test runs at multiple cache sizes via run_with_various_cache_sizes.
Add 3 tests designed to catch stale-cache-after-save bugs if save_and_cache_node is implemented in the future: - saved leaf other entries readable (lazy ByRef survives save) - overwrite varying value sizes (layout changes vs cached offsets) - rebalance then read siblings (rotation/merge vs cached neighbors) All pass today with invalidate-on-save and serve as acceptance criteria for the optimization.
Add cached variants of 7 important existing tests at cache=1 (max collisions) and cache=64 (warm cache). Uses u32/u64 types only to keep runtime bounded — the originals already cover the full type grid. Tests covered: insert+get, insert overwrites, insert+remove many, pop_first, pop_last, first_key_value, last_key_value.
Verify the cache is actually used and metrics are consistent: - hits > 0 after get-heavy workload with cache enabled - hits == 0 and misses == 0 with cache disabled (0 slots) - metrics reset after clear_new()
…ring - Add non-power-of-two cache sizes (3, 7, 50) to proptests, run_with_various_cache_sizes, and the equivalence test. Users can pass any value to with_node_cache, so correctness must not depend on power-of-two alignment. - Cache metrics tests now explicitly call node_cache_clear_metrics() before the measured workload so counters reflect only that workload. - Improve doc comments on NodeCacheMetrics and node_cache_clear_metrics to explain that counters accumulate and must be cleared before measuring a specific workload.
Add 5 tests that exercise the pattern: populate tree, read all keys (warming the cache), modify the tree, then read again. This catches bugs where a write invalidates only the nodes it touches but leaves other (now-stale) nodes in the cache from the read phase. - read-warm then insert (with splits) - read-warm then overwrite - read-warm then remove (with merges) - read-warm then pop_first/pop_last - repeated read-write cycles
|
|
|
|
|
|
- get_miss: non-existent keys with warm cache - get_then_remove: get warms path, remove modifies same path - get_then_insert: get warms path, overwrite modifies same path - contains_then_remove: contains warms path without reading value - peek_then_pop: last_key_value/first_key_value then pop - deeper_tree: 2000 entries for deeper cache pressure
- cache_resize_mid_use: resize cache between operations (16 → 1 → 0 → 64), verify correctness after each resize - cache_partial_iter_then_mutate: partial iteration warms cache, then mutations modify the tree, then full iteration must be consistent
maksymar
changed the title
maksymar
force-pushed
the
maksym/btm-tests
branch
from
2011846 to
abc9dd8
Compare
Mix get, contains_key, first/last_key_value on the same cached nodes between bulk overwrites across multiple rounds.
maksymar
force-pushed
the
maksym/btm-tests
branch
from
0b922c2 to
34db620
Compare
maksymar
changed the title
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 PR adds new tests covering node cache correctness gaps.
The existing suite had minimal cache-specific coverage — one
proptestat a single cache size.These tests verify: