perf(convert): intern decode-path dict keys - #21
Merged
Merged
Conversation
- opt!/req!/opt_with!/req_with! macros and the standalone get_item calls in py_to_annotation/py_to_segmentation/py_to_rle now fetch each key via pyo3::intern! instead of a bare &str, which allocated a fresh PyString on every call; macro $key tightened from :expr to :literal to match what intern! requires - decoding an RF-DETR-shaped annotation list calls get_item once per field per detection, so this removes millions of throwaway PyString allocations for a fixed set of ~10 field names; loadRes(list of dicts) routes through the same py_to_annotation and benefits too - COCO(dict) construction is 18-20% faster across three shapes (small dict, 1.5M-annotation RLE segmentation dict, bbox-only dict), thread-count-invariant since the decode loop is sequential - new TestKnownKeysRoundTrip round-trips every known annotation/image/ category field through COCO(dict) and fails if an interned key literal drifts from its field, proven by injection on both a required-key typo (raises) and an optional-key typo (silently drops the field) - 10/10 bbox + 4/4 segm bit-identical against a fresh main build; no numeric or structural change to the decoded dataset - extract_extra's key-to-String allocation and linear known-key scan is not touched here; reprofiling found it costs roughly half of what remains of decode time after this change, left open Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
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 pull request optimizes the deserialization of Python dicts into datasets by interning field-name keys, significantly reducing string allocations and improving performance. All macro and manual dict field lookups in Rust now use
pyo3::intern!, ensuring each key string is created only once per process. The change is validated by a comprehensive new test that round-trips every known field of annotations, images, and categories, ensuring no key is missed or misnamed.Performance and memory improvements:
crates/hotcoco-pyo3/src/convert.rsnow usepyo3::intern!to intern field-name keys, preventing millions of temporary string allocations during dataset decoding and yielding an 18–20% speedup for common workloads. [1] [2] [3] [4] [5] [6]Testing and validation:
TestKnownKeysRoundTripincrates/hotcoco-pyo3/tests/test_dropin_gaps.py, which round-trips every known annotation, image, and category field throughCOCO(dict), ensuring all keys are correctly interned and decoded, with failures pinned for both required and optional fields.Documentation and changelog:
CHANGELOG.mdto document the key interning optimization, its performance impact, and the new test coverage.These changes collectively improve dataset decoding efficiency and reliability, especially for large annotation sets.