Dict lookup fast path (perf track 2)

[samuelcolvin]

Dict lookup without candidate-key clone

find_index_hash now compares simple keys (str/bytes/int/bool/float/big int, interned or heap) directly against the borrowed stored key via the existing one-sided eq_str/eq_i64/eq_f64/eq_bigint/eq_bytes helpers (which take &VM immutably), skipping the per-probe clone_with_heap + defer_drop + py_eq round trip that existed only to satisfy the borrow checker. Candidates where either side is non-simple (instances, tuples, None, …) keep the clone + py_eq slow path, so reflected-__eq__ semantics are unchanged. Benefits getitem, setitem, in, and dict.get alike.

The eq_* helpers and simple_key_eq/is_simple_key are marked #[inline]: on the instrumented CodSpeed build they stayed outlined and the per-candidate call overhead showed up as a find_index_hash self-time regression in the dict-heavy flamegraphs (neutral on local builds, which already inline them).

Two heavier restructurings were tried and measured consistently worse locally, so they were deliberately not taken: equality inside the hashbrown probe closure (~5% slower on dict_comp — the fat closure hurts codegen and the single-group SIMD probe makes early exit worthless) and a hoisted borrowed-key enum (~2% slower — enum materialization costs more than the double-match it removes).

Tests

New cross-type dict-key section in test_cases/dict__ops.py: int/bool/float/bigint key equivalence (incl. exact 2.0**100 ↔ 2**100), bool overwrite keeping the original key object, runtime-built str keys, distinct int/str/bytes/tuple keys, NaN keys. Verified against both CPython and Monty.

Green locally: test-cases, test-memory-model-checks, test-ref-count-return, test-no-features, lint-rs, lint-py.

Benchmarks

Measured while this PR still included track 3 (CodSpeed vs main): agg_rows −7.9%, with the dict lookup being the dominant contributor per the flamegraphs (the probe's clone+py_eq path disappeared). The next CodSpeed run on this branch gives the isolated number.

Note

This PR originally also carried track 3 (string cheap wins: concat_allocate_str, the py_hash hot-arm rewrite, borrowed startswith/endswith affixes). That's been split out to the perf-wins-004 branch so each change can be reviewed and measured on its own.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QBcn8RK9Wi4BG3b2aWN4rD

---

Summary by cubic

Speeds up dict lookups by adding a simple-key fast path that compares str/bytes/int/bool/float/big int keys in place and skips per-probe cloning. Non-simple keys still use py_eq to preserve reflected __eq__; local agg_rows −16%.

• Refactors
  • find_index_hash: in-place compare for simple keys via inlined eq_*; skip clone + defer_drop; fallback keeps reflected equality.
  • Marked eq_* helpers #[inline]; expanded dict-key tests (numeric-tower equivalence, runtime-built vs literal str, NaN, key-type separation).

^{Written for commit ec08c2a. Summary will update on new commits.}

[codspeed-hq]

Merging this PR will not alter performance

✅ 26 untouched benchmarks
⏩ 16 skipped benchmarks¹

---

_{Comparing perf-wins-003 (ec08c2a) with main (5de8e00)}

Footnotes

1. 16 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[github-actions]

Codecov Results 📊

❌ Patch coverage is 0.00%. Project has 40869 uncovered lines.
✅ Project coverage is 49.65%. Comparing base (base) to head (head).

Files with missing lines (1)

File	Patch %	Lines
crates/monty/src/types/dict.rs	0.00%	⚠️ 33 Missing

Coverage diff

@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
+ Coverage    49.65%    49.65%        —%
==========================================
  Files          316       316         —
  Lines        81113     81169       +56
  Branches    172509    172661      +152
==========================================
+ Hits         40276     40300       +24
- Misses       40837     40869       +32
- Partials      3097      3097         —

---

Generated by Codecov Action

[codecov]

Codecov Report

❌ Patch coverage is 81.81818% with 6 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
crates/monty/src/types/dict.rs	81.81%	5 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[cubic-dev-ai]

1 issue found across 7 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crates/monty/src/types/dict.rs">

<violation number="1" location="crates/monty/src/types/dict.rs:267">
P2: Dict lookup can miss equal big-int keys when the stored key is `InternLongInt`, because the new simple-key path treats that representation as "simple" but converts `NotImplemented` to `false` instead of falling back to reflected `py_eq`. Consider excluding `InternLongInt` from `is_simple_key` (or teaching the one-sided helpers to compare against it) so this path preserves existing equality semantics.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Dict lookup can miss equal big-int keys when the stored key is InternLongInt, because the new simple-key path treats that representation as "simple" but converts NotImplemented to false instead of falling back to reflected py_eq. Consider excluding InternLongInt from is_simple_key (or teaching the one-sided helpers to compare against it) so this path preserves existing equality semantics.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At crates/monty/src/types/dict.rs, line 267:

<comment>Dict lookup can miss equal big-int keys when the stored key is `InternLongInt`, because the new simple-key path treats that representation as "simple" but converts `NotImplemented` to `false` instead of falling back to reflected `py_eq`. Consider excluding `InternLongInt` from `is_simple_key` (or teaching the one-sided helpers to compare against it) so this path preserves existing equality semantics.</comment>

<file context>
@@ -222,6 +222,57 @@ fn json_key_equals_str(key: &Value, expected: &str, heap: &Heap<impl ResourceTra
+        | Value::Float(_)
+        | Value::InternString(_)
+        | Value::InternBytes(_)
+        | Value::InternLongInt(_) => true,
+        Value::Ref(id) => matches!(
+            vm.heap.get(*id),
</file context>

[samuelcolvin]

Reviewed the first CodSpeed run (6a4bb88b vs main 6a4ba434) via the flamegraphs:

Wins confirmed — find_index_hash now dispatches through simple_key_eq/eq_str with no per-probe clone (agg_rows 59.3→55.9ms, −5.7% instrumented; −16% local wall-time), concat_allocate_str replaced the format! machinery, and loop_mod_13 improved −10.3%. str_parse −4.5%.

Regression found and reverted — the dec_ref work-stack pooling was net negative: the mem::take/restore ran on every dec_ref call (most only decrement and free nothing), while the Vec::new() it replaced is free until children are actually pushed. json_dumps dec_ref went 3.5→4.6ms (+4.9% on the bench, replace<Vec<HeapId>> alone 2.8%); agg_rows dec_ref 7.6→7.9ms despite the dict win. Reverted in 31fd512 — heap.rs is back to main plus a comment recording the finding.

[samuelcolvin]

CodSpeed run on the revert commit (6a4bbdc5) confirms it: the dec_ref-pooling regressions are gone — json_dumps 34→32.8ms (now +1.2% vs main, within cross-CPU noise), pair_tuples/builtin_args/list_append_int back to parity — while the tracks-2/3 wins held or improved slightly: agg_rows −7.9%, loop_mod_13 −10.7%, str_parse −5.6% vs main, with re_extract −3.5% and fstring_report −1.6% as side benefits. fib unchanged as expected (that's track 4). Note both PR runs executed on a different CPU model than the main baseline run, so sub-2% deltas aren't meaningful.

[cubic-dev-ai]

2 issues found across 5 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crates/monty/src/types/str.rs">

<violation number="1">
P3: `startswith`/`endswith` now copy every prefix or suffix before checking it, so large tuple affixes pay avoidable allocations even when the first item matches. Consider preserving the existing args guard and validating/iterating borrowed interned or heap strings instead of returning `Vec<String>`.</violation>

<violation number="2">
P3: Heap-string concatenation now does two extra full-string allocations before building the result. Borrowing both operands and filling one `String::with_capacity(self.len() + other.len())` keeps `str + str` on the cheap path without changing semantics.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[davidhewitt]

Seems generally a good idea, potentially worth applying to other operations like sets, and maybe even in list methods like count or index.

[davidhewitt]

FWIW this sort of thing might be better in the settings.local.json or your user-level settings.