From 29f869aba068eb8cdd03f57b05970f0e7517efa0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 31 Jul 2026 03:09:36 +0000 Subject: [PATCH 1/2] fix(benchmarks): don't measure hit-phase timing against evicted keys SizeLimitedCache evicts uniformly at random (SizeLimitedMixin._pick_eviction_target), so populating a cache smaller than `iterations` with unique keys doesn't guarantee any particular subset survives. benchmark_integration's "contains hit" and "hit" phases blindly reused the full original arg list, silently re-measuring misses (and re-inserts) for keys evicted during the miss phase. Restrict both phases to args confirmed resident via `func.contains()` right after the miss phase, and report the actual sample size used. Refs #150. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Mzs4Bw7dsBUC4bXhhjWmoQ --- benchmarks/benchmark_integration.py | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/benchmarks/benchmark_integration.py b/benchmarks/benchmark_integration.py index 1424220c..77ab26e8 100644 --- a/benchmarks/benchmark_integration.py +++ b/benchmarks/benchmark_integration.py @@ -107,6 +107,19 @@ def benchmark_integration(name, cache_obj, func, args, iterations=10): } ) + # SizeLimitedCache evicts uniformly at random (see + # SizeLimitedMixin._pick_eviction_target), so populating a cache smaller + # than ``iterations`` with unique keys does not guarantee any particular + # subset of them survives. The "hit"-style phases below must only reuse + # args that are actually still resident, otherwise they silently + # re-measure misses (and re-inserts) for the evicted ones. + resident_args = [] + with cache(cache_obj): + for i in range(iterations): + arg = args[i] if isinstance(args, list) else i + if func.contains(arg): + resident_args.append(arg) + # 2. Contains (Cache Check - Miss) contains_miss_times = [] for i in range(iterations): @@ -133,8 +146,7 @@ def benchmark_integration(name, cache_obj, func, args, iterations=10): # 3. Contains (Cache Check - Hit) contains_hit_times = [] - for i in range(iterations): - arg = args[i] if isinstance(args, list) else i + for arg in resident_args: # Use .contains method exposed by @fleche to get the key and pass to contains start = time.perf_counter() with cache(cache_obj): @@ -146,17 +158,15 @@ def benchmark_integration(name, cache_obj, func, args, iterations=10): { "benchmark": "integration_contains_hit", "name": name, - "iterations": iterations, + "iterations": len(resident_args), "time": min(contains_hit_times), } ) - # 4. Second Call (Hit) + # 4. Second Call (Hit). Only args still resident after the miss phase + # (see ``resident_args`` above) are reused here. hit_times = [] - # Reuse the same args from above, they are now in cache - for i in range(iterations): - arg = args[i] if isinstance(args, list) else i - + for arg in resident_args: start = time.perf_counter() with cache(cache_obj): func(arg) @@ -167,7 +177,7 @@ def benchmark_integration(name, cache_obj, func, args, iterations=10): { "benchmark": "integration_hit", "name": name, - "iterations": iterations, + "iterations": len(resident_args), "time": min(hit_times), } ) From 1773786b7b54defa26e7a65defc326565650b880 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Fri, 31 Jul 2026 21:24:06 +0000 Subject: [PATCH 2/2] fix(benchmarks): size the SizeLimitedCache config above the workload The SizeLimitedCache(max=10) config could never measure hits: the cache instance is shared by all three benchmark functions, so it accumulates 3 * 20 == 60 call records while keeping only 10. Since SizeLimitedMixin._pick_eviction_target evicts uniformly at random, the "contains hit" and "hit" phases re-measured misses (recompute + re-save) for the ~50 evicted keys. Size the config above the workload instead of filtering the hit phases: max_size=100 already exceeds the 60 keys inserted, so nothing is evicted and every hit-phase call is a real hit. The deliberately-undersized variant is dropped (it would otherwise duplicate the max=100 config once enlarged), and the benchmark loop is back to its original form. Co-authored-by: Marvin Poul <2719909+pmrv@users.noreply.github.com> --- benchmarks/benchmark_integration.py | 41 +++++++++++------------------ 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/benchmarks/benchmark_integration.py b/benchmarks/benchmark_integration.py index 77ab26e8..34448e89 100644 --- a/benchmarks/benchmark_integration.py +++ b/benchmarks/benchmark_integration.py @@ -107,19 +107,6 @@ def benchmark_integration(name, cache_obj, func, args, iterations=10): } ) - # SizeLimitedCache evicts uniformly at random (see - # SizeLimitedMixin._pick_eviction_target), so populating a cache smaller - # than ``iterations`` with unique keys does not guarantee any particular - # subset of them survives. The "hit"-style phases below must only reuse - # args that are actually still resident, otherwise they silently - # re-measure misses (and re-inserts) for the evicted ones. - resident_args = [] - with cache(cache_obj): - for i in range(iterations): - arg = args[i] if isinstance(args, list) else i - if func.contains(arg): - resident_args.append(arg) - # 2. Contains (Cache Check - Miss) contains_miss_times = [] for i in range(iterations): @@ -146,7 +133,8 @@ def benchmark_integration(name, cache_obj, func, args, iterations=10): # 3. Contains (Cache Check - Hit) contains_hit_times = [] - for arg in resident_args: + for i in range(iterations): + arg = args[i] if isinstance(args, list) else i # Use .contains method exposed by @fleche to get the key and pass to contains start = time.perf_counter() with cache(cache_obj): @@ -158,15 +146,17 @@ def benchmark_integration(name, cache_obj, func, args, iterations=10): { "benchmark": "integration_contains_hit", "name": name, - "iterations": len(resident_args), + "iterations": iterations, "time": min(contains_hit_times), } ) - # 4. Second Call (Hit). Only args still resident after the miss phase - # (see ``resident_args`` above) are reused here. + # 4. Second Call (Hit) hit_times = [] - for arg in resident_args: + # Reuse the same args from above, they are now in cache + for i in range(iterations): + arg = args[i] if isinstance(args, list) else i + start = time.perf_counter() with cache(cache_obj): func(arg) @@ -177,7 +167,7 @@ def benchmark_integration(name, cache_obj, func, args, iterations=10): { "benchmark": "integration_hit", "name": name, - "iterations": len(resident_args), + "iterations": iterations, "time": min(hit_times), } ) @@ -211,12 +201,13 @@ def main(): Sql(f"sqlite:///{tmp_dir}/db_h5.sqlite"), ), ), - # SizeLimitedCache: max_size smaller than iterations → evictions occur - ( - "SizeLimitedCache(Memory,max=10)", - SizeLimitedCache(ValueMemory({}), CallMemory({}), max_size=10), - ), - # SizeLimitedCache: max_size larger than iterations → no evictions + # SizeLimitedCache. ``max_size`` must stay above the total number + # of calls this config accumulates — the instance is shared by all + # three functions below, so it ends up holding + # 3 * iterations == 60 call records. Anything smaller triggers + # evictions, and because SizeLimitedMixin._pick_eviction_target + # evicts uniformly at random there is no way to tell which keys + # survive; the hit phases would then silently re-measure misses. ( "SizeLimitedCache(Memory,max=100)", SizeLimitedCache(ValueMemory({}), CallMemory({}), max_size=100),