Skip to content

Avoid copying entire mappings during type inference - #7057

Draft
Alek99 wants to merge 1 commit into
mainfrom
codex/perf-bounded-mapping-inference
Draft

Avoid copying entire mappings during type inference#7057
Alek99 wants to merge 1 commit into
mainfrom
codex/perf-bounded-mapping-inference

Conversation

@Alek99

@Alek99 Alek99 commented Sep 7, 2026

Copy link
Copy Markdown
Member

Mapping type inference samples only 100 keys and values, but currently materializes every key and value before taking that sample. Use bounded iteration so inference keeps the same sample and resulting types without copying or retrieving the rest of a large mapping.

On a local CPython 3.14.5 / Apple M5 Pro benchmark, inference for a one-million-entry dict fell from 5.62 ms to 0.026 ms, with peak traced temporary allocation falling from 8 MB to 360 B. This measures the inference helper, not whole-app compilation. Added a CodSpeed benchmark across mapping sizes and regressions covering empty mappings, the exact sample boundary, and custom Mapping iteration/value access. The bounded-iteration regression fails on the unchanged base.

Validation:

  • Full unit suite: 8,303 passed, 18 skipped; 75.95% coverage.
  • Focused Var tests and new benchmark: 260 passed.
  • Repository-wide Ruff check and format check passed.
  • pyright reflex tests and changed-file type checks passed.

Independent of the other performance changes; based directly on main.

Review in cubic

@greptile-apps

greptile-apps Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR bounds mapping type inference to the first 100 keys and values without materializing complete mapping views.

  • Replaces full key and value copies with bounded islice iteration.
  • Adds regressions for empty mappings, the 100-item boundary, and custom mapping access counts.
  • Adds CodSpeed coverage across several mapping sizes.
  • Adds the required reflex-base performance news fragment.

Confidence Score: 5/5

The PR appears safe to merge, with bounded iteration preserving the established type-inference sample for supported mappings.

No actionable correctness, security, integration, or repository-rule issues remain; the implementation preserves first-100 sampling semantics and the tests cover the changed boundary behavior.

Important Files Changed

Filename Overview
packages/reflex-base/src/reflex_base/vars/base.py Preserves the existing 100-item inference sample while avoiding work and allocation proportional to the entire mapping.
tests/units/reflex_base/vars/test_base.py Verifies inferred types, exact sample boundaries, empty mappings, and bounded custom-mapping iteration and value access.
tests/benchmarks/test_type_inference.py Adds a correctly integrated CodSpeed benchmark that measures inference independently of mapping construction.
packages/reflex-base/news/+bounded-mapping-inference.performance.md Documents the downstream compilation-performance improvement in the package news format.

Reviews (1): Last reviewed commit: "perf: bound mapping type inference to th..." | Re-trigger Greptile

@codspeed-hq

codspeed-hq Bot commented Sep 7, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

βœ… 32 untouched benchmarks
πŸ†• 3 new benchmarks
⏩ 8 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
πŸ†• test_mapping_type_inference[100] N/A 589.5 Β΅s N/A
πŸ†• test_mapping_type_inference[10000] N/A 588.8 Β΅s N/A
πŸ†• test_mapping_type_inference[100000] N/A 590.4 Β΅s N/A

Comparing codex/perf-bounded-mapping-inference (31905d7) with main (c49a85d)

Open in CodSpeed

Footnotes

  1. 8 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. ↩

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant