Skip to content

Memoize parse-pattern builds across instances; reuse one parser per locale - #1398

Closed
John-Cusack wants to merge 1 commit into
arrow-py:masterfrom
John-Cusack:memoize-parse-pattern
Closed

John-Cusack wants to merge 1 commit into
arrow-py:masterfrom
John-Cusack:memoize-parse-pattern

Conversation

@John-Cusack

Copy link
Copy Markdown

Pull Request Checklist

  • 🧪 Added tests for changed code.
  • 🛠️ All tests pass when run locally (run tox or make test to find out!).
  • 🧹 All linting checks pass when run locally (run tox -e lint or make lint to find out!).
  • 📚 Updated documentation for changed code.
  • ⏩ Code is up-to-date with the master branch.

No documentation update: there is no public API change — arrow.get still
builds the same parsers with the same results; the memo is internal.

Description of Changes

Every arrow.get(s) / arrow.get(s, fmt) constructs a fresh
DateTimeParser(locale) with cache_size=0, so each parse pays:

  1. DateTimeParser.__init__ — locale lookup + 6 _generate_choice_re →
    re.compile calls, and
  2. _generate_pattern_re(fmt) — a full regex rebuild from the format
    string (the top cProfile frame of a format parse).

The existing lru_cache opt-in (cache_size > 0, default 0) cannot help
this path: it wraps the bound method per instance, and the factory
discards the instance after one call.

This PR:

  • memoizes _generate_pattern_re at class level in arrow/parser.py
    (bounded at 128 entries, GIL-atomic dict access under a lock for
    eviction). The key embeds a fingerprint of the parser's effective
    input map (parser class + locale name + token→pattern/flags snapshot
    built in __init__), so per-instance or per-subclass map
    customizations — including the test_parse_unrecognized_token
    _BASE_INPUT_RE_MAP mock — can never read another parser's entry.
    Compiled patterns are immutable and (map, fmt)-determined, so the
    cache is semantically transparent: identical re objects, identical
    matches and errors.
  • reuses one DateTimeParser per locale string in the factory
    (arrow/factory.py _get_parser), so __init__'s locale-regex
    rebuild also runs once per locale instead of once per call. Parsers
    are effectively immutable after construction for stock locales.

Measured

Ryzen 9 5950X, CPython 3.12.3, frozen 2000-item batches
(YYYY-MM-DD HH:mm:ss format batch + ISO-8601 batch), each arm a fresh
subprocess, in-process min-of-5 after warmup, min across 3 processes;
peak RSS via /proc VmHWM (this box's ru_maxrss is constant-broken —
verified against VmHWM, identical values both arms):

Track Stock (master @ 2224255) Patched Speedup
arrow.get(s, FMT) × 2000 0.0441 s 0.0114 s 3.86x
arrow.get(s) ISO × 2000 0.0652 s 0.0228 s 2.86x
representative pipeline (parse→shift→format→humanize) × 2000 0.0709 s 0.0391 s 1.81x

Peak RSS flat across arms (~19.0 MB both). Outcome counts identical
both arms (all 2000 items parse in every track on both sides).

Reproduce (at 2224255 vs this branch, venv per requirements/requirements-tests.txt):

pip install -r requirements/requirements-tests.txt && pip install -e .
python bench_arrow.py   # frozen 2000-item fmt/iso/pipeline tracks above

(Bench script available on request — say the word and I'll attach it.)

Test / lint proof

Their own gates, venv built per Makefile
(requirements-tests.txt + pip install -e .):

  • pytest: 1908 passed (4 new: cross-instance memo sharing,
    locale isolation en/fr/de, 128-entry eviction bound + transparent
    rebuild, factory one-parser-per-locale). Pre-existing caching tests
    (test_parser_no_caching, test_parser_1_line_caching, ...) and the
    _BASE_INPUT_RE_MAP mock test pass unmodified.
  • Coverage gate (--cov-branch --cov=arrow --cov-fail-under=99, as in
    tox.ini): 99.93%, required 99% reached.
  • pre-commit run --all-files: all hooks pass (black, isort,
    flake8 + bugbear + annotations, mypy, pyupgrade).
  • Differential equivalence: 20-case corpus (ISO, 14 format shapes incl.
    locale month/day names, week dates, meridiem, fractional seconds, plus
    5 error-path cases) through arrow.get — stock vs patched
    20/20 identical, including exception types and messages.

…ocale

Every arrow.get(s)/arrow.get(s, fmt) constructed a fresh DateTimeParser,
so its __init__ locale-regex rebuild and _generate_pattern_re format
rebuild ran per call while the opt-in per-instance lru_cache could never
hit on the factory path. Cache the compiled (tokens, pattern) at class
level keyed by (parser class, locale, input-map fingerprint, fmt), and
keep one DateTimeParser per locale string in the factory.

fmt-parse 3.86x, iso-parse 2.86x, representative pipeline 1.81x;
peak RSS flat. Full suite 1908 passed, coverage gate holds, pre-commit
clean. No behavior change: 20-case differential stock-vs-patched
identical.
@codecov

codecov Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (2224255) to head (af2999f).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #1398   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines         2315      2342   +27     
  Branches       358       360    +2     
=========================================
+ Hits          2315      2342   +27     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@John-Cusack

Copy link
Copy Markdown
Author

CI triage note (for reviewers): 24/27 legs green, including Linting and all CPython 3.10–3.14 legs. The 3 reds are not from this change:

  • windows-latest (pypy-3.11) fails its tox run identically on unmodified master (2224255 — visible across several master check-suite runs), so pre-existing.
  • ubuntu-latest (3.8) / ubuntu-latest (3.9) fail in the "Set up Python" step before any test executes (EOL-Python provisioning), likewise unrelated to this diff.

Happy to rebase/retest if you want a fresh full-green signal.

@John-Cusack

Copy link
Copy Markdown
Author

Closing this. The shared pattern cache is keyed on a snapshot of the input map taken in __init__. If a parser's _input_re_map is changed after that, it and a stock arrow.get("2013", "YYYY") can break each other: whichever parses YYYY first fills the cache entry, and the other then fails. The gain for a real caller is also small: in ics 0.7.3, parsing a generated 500-event calendar was less than 5% faster.

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