[ybai08]

---

See #108.

Patterns taken from the metadata are compiled with re.compile() on every call (leading digits, format patterns, national number patterns, IDD and national prefixes). This relies on the re module's cache, which holds 512 entries, while the metadata has about 2,000 distinct patterns. An application that handles numbers from many regions keeps evicting patterns and compiling them again.

This adds re_util.compile_pattern(), which stores compiled patterns keyed by their pattern string, much like the Java code's RegexCache. Unlike RegexCache, which is an LRU cache with 100 entries in PhoneNumberUtil and 64 in AsYouTypeFormatter, this cache has no size limit. A limit that small would thrash across many regions just as re's cache does. An LRU limit can be added if a bound is preferred. All the places that compiled a metadata pattern now use it: phonenumberutil.py, asyoutypeformatter.py, and the alternate-format check in phonenumbermatcher.py. The key is the pattern string rather than the metadata object, so the cache stays correct if a metadata object is changed. Its size is limited by the metadata: compiling every distinct pattern in the regional metadata took about 1.2 MB (measured with tracemalloc). The stub for the new function is in re_util.pyi. No generated files are touched.

Two new tests (testMetadataPatternsCompiledOnce in phonenumberutiltest.py, testAYTFMetadataPatternsCompiledOnce in asyoutypetest.py) run a set of operations once. They then wrap re.compile and check that running the same operations again compiles nothing. On dev without this change, both tests fail and list the metadata patterns that get recompiled.

Timings (CPython 3.12, macOS arm64, real metadata), best of a few runs:

workload	dev	this branch
parse + is_valid_number + number_type + national format for each region's fixed-line and mobile example numbers (489 numbers, 3 passes)	0.150 s	0.015 s
the same calls for 500 US numbers (5 passes)	0.025-0.032 s	0.022-0.023 s

Checks run (CPython 3.12, plus the test suite on 3.9 and 3.14):

• python -m testwrapper (from python/): 310 tests OK on 3.9, 3.12 and 3.14
• pycodestyle --ignore E501,E129,E402,W504 phonenumbers/*.py tests/*.py (the make -C tools/python lint command): clean
• python run_stubtest.py with mypy 0.921 / types-protobuf 3.19.21: exit 0
• mypy --exclude pb2/ -p phonenumbers: no issues

Not run: the PyPy 2.7 / PyPy 3.9 CI jobs, the coverage job, and the metaclean + regenerate step of CI. The change does not touch the generated data. The new code does not use any Python-3-only syntax.

[daviddrysdale]

Thanks for suggesting this; only some minor comments on the code.

However, is this solving a problem that's really observed in the wild? I'd have guessed the Python 512-entry cache would cover most scenarios (and is bigger than the upstream Java code's cache size), unless there's a heavily international set of inputs.

[ybai08]

Thanks. The observation in the wild is the one from #108 (checking many short strings, 6-10x from caching). I don't have a production workload of my own to point to, so here is a measurement of when the re cache stops helping.

One example number per supported region (245) touches 1,472 distinct patterns through parse + is_valid_number + format_number(INTERNATIONAL) + number_type, so mixed-country input cycles through more patterns than re._MAXCACHE (512) and keeps evicting them. Single-country input fits in the cache and sees no difference:

workload (×3)	Python	dev (e921470)	this branch
one example number from each of 245 regions	3.13.15	0.129 s	0.008 s
same count, all US	3.13.15	0.009 s	0.008 s
one example number from each of 245 regions	3.9.6	0.208 s	0.013 s
same count, all US	3.9.6	0.015 s	0.015 s

(macOS arm64, time.perf_counter, after one warm-up pass.) So it matters for international input like #108's and is neutral otherwise. Happy to address the code comments once they're up, or to close this if you'd rather keep relying on the re cache.

[ybai08]

The pypy2.7 job failed on the two new tests (pypy3.9 was then cancelled). On Python < 3.4 there is no re.fullmatch, so re_util.fullmatch compiles a grouped copy of the pattern on every call, and the tests' re.compile spy sees those. a32b047 makes both tests return early on Python < 3.4; the cache itself still applies there, but the fallback's per-call compile is unchanged. Python 2.7 / PyPy weren't available locally, so this relies on CI for those; the suite passes on CPython 3.9 and 3.13.

[daviddrysdale]

Maybe call this re_compile to make it obvious what it behaves like.

[ybai08]

Sorry for the slow follow-up on your three comments. All three are done in 93d7823:

• compile_pattern is now re_compile, in re_util.py, re_util.pyi and every caller.
• The warm-up call in both new tests (asyoutypetest.py and phonenumberutiltest.py) now has the comment you suggested: "Ensure that all of the regexps in the test are already cached, regardless of whether other test cases have used them or not."

testwrapper.py passes: 310 tests on CPython 3.9.6 and on 3.13. run_stubtest.py did not run here. With the mypy and protobuf stubs I have, it stops on errors in the generated phonenumbers/pb2/phonenumber_pb2.py before it reaches the stubs.

[daviddrysdale]

Thanks for the updates, the code looks good (modulo trivial lint warnings).

However, I'm still wondering whether we really need an extra unbounded cache in front of the built-in re.compile cache – the places where it has an effect seem a bit artificial.

Here's an idea, though: could we make use of the extra cache optional, so re_compile just falls through to re.compile when the option isn't set?

[ybai08]

Thanks. The lint warnings are fixed in c86b77c: one E127 from the shorter re_compile name, and two E306 in the new tests. make -C tools/python lint's pycodestyle command is clean now, and testwrapper.py passes.

Optional sounds good. My proposal:

• The cache is off by default, and re_compile is then re.compile.
• One call turns it on, for example phonenumbers.re_util.enable_pattern_cache(), with a matching stub entry.
• One sentence in the README section that already lists PhoneMetadata.load_all() for start-of-day loading: "If you check many numbers from many regions, phonenumbers.re_util.enable_pattern_cache() keeps every compiled metadata pattern, so they are not recompiled once re's own cache is full."
• The two tests then enable the cache and turn it off again afterwards, and a third checks that re_compile is re.compile while the cache is off.

Would you prefer a different name or place for the switch, for example a keyword argument on load_all()? If so, tell me which.