| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Here's the performance diff I see locally: exercises.py:normalize_perf: 332 nsec (+239 nsec, 257%) |
Sorry, something went wrong.
|
Looking at the Ubuntu 3.14 run, I see something similar: exercises.py:normalize_perf: 420 nsec (+310 nsec, 282%) So the performance differences are real, but still in the nsec range (sub micro-second benefit). Let's see if we can quantify the value proposition, specifically by estimating the number of times this call is made globally. If we can't identify a real value proposition, the simpler implementation should prevail. |
Sorry, something went wrong.
|
Following up on the value-proposition question, here's what I can piece together about how often Prepared.normalize actually runs in the wild and what it's worth. I've tried to make every assumption explicit; corrections welcome. 1. What the upstream change was justified onThe CPython PRs justify the change with a throughput micro-benchmark, not a workload: python/cpython#143660 normalizes all 8,344,947 PyPI names in 5.15 s → 1.38 s (3.7×); #144083 just compares the three implementations across Python versions. The rationale was delegated to the blog — but the blog's real-world numbers are about packaging's Version objects ("creating Versions over 4.8 million times" in one resolution), not the name-normalization function. It offers no call-frequency data for canonicalize_name/normalize itself. The equivalent value question I raised in python/cpython#143658 went unanswered. 2. Where normalize is actually called (measured, Python 3.14, env with N=94 dists)Lookup.__init__ normalizes every installed dist-info name when it builds its index, and the index is built once per process (memoized on directory mtime). So per cold process:
The structural point: even a single version() lookup normalizes all installed names. Cost is O(N in the environment) per process, so normalize_perf (one call) understates per-process cost by ~N×. 3. Per-call deltaCleanly (locally bound, 4 representative names, Apple Silicon / 3.14): re.sub 529 ns → .lower().replace() 121 ns, Δ ≈ 414 ns/call (4.4×). Consistent with my earlier ~240–310 ns here and CPython's ~450 ns. 4. …but the operations are I/O-bound, so it's invisibleCold operation wall-time vs. the normalize budget inside it (same machine, N=94):
The saving (Δ × N) is ~2–4% of discovery at most, <1% of entry_points/version. The rest is listdir/stat on site-packages and reading entry_points.txt/METADATA. This matches the exercises.py suite: on the revert, the only benchmark that moved was normalize_perf itself (332 ns / 420 ns in my two runs). discovery (~173 µs), cached/uncached distribution (~169/273 µs), and entry_points() (~2.3 ms) did not register a change — the micro-op disappears into filesystem noise. 5. The pip case, specifically (since it's the obvious "hot caller")I instrumented Prepared.normalize and ran real pip commands (pip uses the importlib backend here):
So pip pays ~N calls once per invocation that enumerates the environment (via importlib.metadata.distributions()), and zero in its resolution/name-matching hot loop — that runs on packaging.canonicalize_name (92 call sites in pip/_internal), a different function that was optimized separately (pypa/packaging#1030). The "large resolution" scenario the blog uses to motivate the change does not exercise importlib.metadata.normalize at all. For pip, this is a flat ~94 × 414 ns ≈ 40 µs, once, inside a multi-hundred-ms process. 6. Aggregate, in the wild (Fermi — assumptions explicit)Anchoring on real data instead of guessing: normalizing all 8.3 M PyPI names saves ~3.78 s (the CPython benchmark delta). So you'd have to normalize the entirety of PyPI ~960 times to recoup one CPU-hour. If I push a global figure anyway:
→ ~5×10¹²–5×10¹³ calls/yr → at 414 ns, ~24 to ~240 CPU-days per year spread across all Python usage on Earth. Real, but per-invocation it's microseconds, inside operations already costing milliseconds of I/O. ConclusionThis is a genuine 4.4× speedup of a function that isn't on any measurable hot path. Its cost is O(N) per process, yet still 1–4% of the I/O-bound operation containing it and invisible in every operation-level benchmark; even pip's dependency resolution doesn't route through it. The aggregate global saving is plausibly tens of CPU-days/year under generous assumptions — genuine but negligible per user, and unmeasurable against filesystem cost. Absent a concrete hot-path use case (which the blog, the CPython issue, and the reviews did not produce), I lean toward the simpler re.sub for readability/maintainability, keeping the performance and unit tests so the trade-off stays measurable. Happy to be shown a workload where this actually matters. Measurements gathered with Claude Code; methods/scripts available on request. |
Sorry, something went wrong.
|
@henryiii and @hugovk , I don't feel like the justification case is strong, so I'd like to revert this to its prior, direct, more concise, single-expression, debt-free implementation using higher-level constructs. I'll let this sit for a while to give a chance to make the case (DO NOT MERGE before 2026-08-31), but my instinct is that we should only hyper optimize where the case is clear. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Reverts the importlib_metadata/__init__.py Prepared.normalize() changes introduced by the merge 8c5d91b — the str.translate/str.replace optimizations from python/cpython#143660 (3e7f3ac) and python/cpython#144083 (001db0d), along with 27169dc's docstring note — restoring the original re.sub-based implementation:
Only __init__.py is touched; the behavioral tests added alongside those commits remain and pass against this implementation.
Closes #540
🤖 Generated with Claude Code