perf(coco): intern annotation field keys

[Borda]

parse_py_annotation looked every field up by passing a C string literal to dict::contains and then again to operator[]. pybind11 builds a fresh Python str for each of those: allocate, decode UTF-8, hash with siphash24 (a new str carries no cached hash), then free. Seven fields, twice each, for every annotation. Interning the keys once removes all of it.

Metric	Before	After	Change
evaluate()	0.81026 s	0.66021 s	1.2273x, -18.52%
Peak RSS	1178.05 MiB	1188.19 MiB	+0.86% (gate: 2%)

Evaluator output digest — SHA-256 over the full precision, recall and scores tensors plus summary stats — is unchanged.

Why this function

A sampled profile (sample, 25 s over a continuous evaluate() loop) put get_cpp_instances — the serial, GIL-held prologue of EvaluateImages, which calls this parser — at roughly 78% of the native evaluate call. The matching loops that look expensive on paper run across ~14 workers and are nearly idle by comparison: ~75 samples each against 20,436 on the main thread. The bottleneck was never the matcher; it was converting annotation dicts, serially, with the GIL held.

Re-profiling after the change confirms the mechanism rather than just the timing:

Symbol	Before	After
pybind11::dict::contains	2942	0
siphash24	890	1
unicode_decode_utf8	1369	1

Both profiles are saved under .reports/analysis/phase0-profile/.

What changed

Seven field names are interned once via PyUnicode_InternFromString and reused for every lookup, so each probe becomes a plain PyDict_GetItem against a key with a cached hash. Each contains-then-subscript pair collapses into one PyDict_GetItemWithError call, halving the lookups.

Behaviour is deliberately preserved, not tidied: each field stays best-effort, so an unconvertible value leaves that field at its default rather than propagating; is_crowd still takes precedence over iscrowd, including when both keys are present. The keys are leaked on purpose — static destructors may run after interpreter teardown without the GIL, and releasing Python objects there is undefined behaviour. Seven small strings is the right price for removing that hazard.

Testing

Full suite 211 passed, 2 skipped; ruff check and ruff format clean. New TestAnnotationFieldParsing covers all five crowd-key spellings, three unconvertible field values, and the all-optional-fields-absent case. Those tests also pass against the unmodified extension, which is what makes them meaningful: they characterise existing behaviour rather than encoding the change. A separate differential fixture — annotations with absent, malformed, and alternate-spelling fields — produced byte-identical digests on both builds.

Latent issue found, not fixed here

While writing the tests I asserted that is_crowd and iscrowd should be interchangeable. They are not, and they were not before this change either: the parser reads is_crowd with an iscrowd fallback, but _prepare derives its ignore flag from iscrowd alone (gt["ignore"] = int(gt.get("iscrowd", 0))). So {"is_crowd": 1} evaluates to AP 1.0 while {"iscrowd": 1} gives -1.0, and when both are present iscrowd decides the outcome despite the parser preferring is_crowd. Identical on baseline and candidate, so it is out of scope here — the new tests pin the current matrix so it cannot drift silently, and the commit body records it. Worth deciding separately whether the is_crowd spelling should be honoured by _prepare too.

Benchmark method

Interleaved A/B over frozen builds: each configuration built, its package directory snapshotted, the two run alternately via PYTHONPATH — 7 outer rounds each, 5 inner per measurement, medians. Sequential before/after runs were avoided after an earlier measurement in this series was confounded by a stale in-tree .so, which inflated an unrelated result by 0.41 s until the baseline was rebuilt from clean sources.

Limits

One host (macOS arm64, Python 3.10.11), one synthetic COCO2017-shaped bbox fixture — not real instances_val2017.json with real predictions. Segmentation and keypoint evaluation share this parser but were not benchmarked; they should gain similarly, since the cost removed is per-annotation and task-independent. RSS spreads overlap between configurations, so the +0.86% is at noise level rather than a measured regression.

Related

Profiling context and the discarded matcher-optimization proposals are in .plans/active/todo_native-matcher-optimization.md. Separately, .reports/analysis/bbox-bottleneck-and-freethreading-2026-08-24.md records that the pinned cibuildwheel builds cp314t wheels by default while neither extension declares Py_MOD_GIL_NOT_USED, so those wheels force the GIL back on process-wide — worth addressing before the next release, independently of this PR. Note that the interned statics here are initialised under the GIL on first call and never mutated, so they are safe as-is and do not add to that work.

[Copilot]

🟡 Changes recommended

Interning failures can crash, and some tests do not exercise the intended native paths.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Optimizes native annotation parsing by reusing interned Python keys and adds regression coverage.

Changes:

• Replaces repeated dictionary lookups with interned-key C API probes.
• Adds annotation parsing behavior tests.

File summaries

File	Description
csrc/faster_eval_api/coco_eval/dataset.cpp	Optimizes annotation field lookup.
tests/test_cpp_safety_regressions.py	Adds parser regression tests.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 3
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.