diff --git a/docs/design/decisions.md b/docs/design/decisions.md index 7acb6674..60e66969 100644 --- a/docs/design/decisions.md +++ b/docs/design/decisions.md @@ -793,6 +793,21 @@ R3's earlier history is under `decisions.md#R2`, which this entry does not repea - 2026-08-29 — and therefore NOT stated in rules.md, which is a deliberate choice rather than an oversight. The document's examples are keyed on input STRINGS, so any statement of the property invites exactly the test that falsifies it — re-case the input, expect the same output — and the counterexamples are already in the corpora. The property is true of the repair given a parse, and rules.md speaks input-to-output; a rule stating it would be over-broad in the one direction a reader would check. What R5's statement says is enough for the promise that IS kept: a mixed-case name is kept unless repair was asked for anyway. That clause was REWORDED for this, and the reword is the whole point rather than a tidy-up. It read `unless repair regardless of how the name is cased was asked for`, which carries two readings -- the intended one, that the request overrides the keeping, and a second one, that the repair disregards the input's casing, which is this property in nearly this bullet's own words. A reader taking the second reading would run the re-casing test predicted above, land on `Velasquez y Garcia, Dr. Juan Q.` (in the corpus today), and conclude the RULE is wrong when only the phrasing was. Nine words, and they asserted the thing the paragraph exists to deny. The property is pinned in tests/test_capitalization.py instead, over names carrying no single-letter word whose class case decides, with `juan y garcia` beside it as the recorded exception. R5's example block gains `"SHIRLEY MACLAINE" → capitalized="Shirley MacLaine"` from this work, and it earns its place on its own ground rather than as half of a convergence pair: it is the only row in the block that fails when the gate is narrowed to lowercase-only, every other row passing that mutation. Measured three ways — gate deleted (passes, so it does not witness the gate's existence), gate narrowed to accept only all-lowercase (FAILS, and alone in the block), Mac/Mc convention deleted (fails, with the other two rows). Until it was added, R5 stated that repair acts on a name written entirely in one case and witnessed only the lowercase half of it. That lowercase half is still `"juan mcdonald"`, which is byte for byte an R4 row as well, and the duplication is deliberate rather than an editing slip: the two rules make different claims about the same line — R4 that the repair honors the Mac/Mc convention, R5 that an all-lowercase name is acted on at all — and dropping it from R5 would leave the gate's lowercase half unwitnessed inside the rule that states the gate. Five other rows already sit under two rules apiece for the same reason (P5/P6 twice, P5/O5, N3/M4, W1/W3). - 2026-08-29 — DEBT this extraction leaves, named so the next commit inherits an obligation rather than a rediscovery. Pulling the gate out into R5 leaves R4 carrying ONE falsehood and ONE ambiguity — different defects wanting different repairs, and `interacts: R5` carries neither, the field being advisory. FALSE: R4 promises repair "vocabulary exceptions (McDonald) included", but `str(parse('Juan Mcdonald').capitalized())` is `'Juan Mcdonald'` — the gate refuses before any vocabulary is consulted, and only `str(parse('Juan Mcdonald').capitalized(force=True))`, `'Juan McDonald'`, reaches the exception. R4 needs its promise scoped to names the gate admits. AMBIGUOUS, not false: R4's "an already-correct name comes back unchanged" means correct by the repair's own conventions, i.e. idempotence, and under that meaning it is true; a reader hears correct as the bearer writes it, and under THAT meaning `str(parse('bell hooks').capitalized())` — `'Bell Hooks'` — looks like a counterexample. It is not one, because `bell hooks` is not already-correct in R4's sense. What R4 owes is a disambiguation of "correct", NOT a narrowing to spare deliberately single-cased names: that would be new behavior, and R5's own rationale declines it on the ground that single case leaves the repair nothing to read. Also for that commit, and inert as things stand: R4's boundary row `"Juan McDonald" → capitalized="Juan McDonald"` passes with R5's gate deleted, exactly like the R5 row that was withdrawn above; rewriting it to `capitalized_forced=` makes it discriminate for R4's own subject but still witnesses nothing about the already-correct question. This commit adds R5 and touches R4 only on its pointer line, leaving both defects as found rather than half-fixed by a commit whose subject is something else. +### parse-cost — what a parse is allowed to cost + +Every number below is a py3.11 measurement of 2026-08-31, recomputable with `uv run python tools/perf/call_count.py` (add `--modules`, `--stages`, or `--against `). That harness is in the tree BECAUSE of this entry: its first draft took figures across several sessions with throwaway scripts on whichever interpreter was to hand, and published a module table that spliced one interpreter's before-column onto another's after-column. Four of its eight module numbers were wrong, its four stage numbers were 2x (2000-parse totals labelled per 1000, which its own arithmetic contradicted — four stages summing past the whole parse), and it blamed the wrong PR. Quote nothing here without the interpreter beside it. + +- 2026-08-31 #475 — the benchmark bounds FUNCTION CALLS per parse, not seconds, as a per-interpreter BAND of ±2%. A loose 5s wall-clock backstop stays over BOTH entry points for what frame counts cannot see: a compiled regex that starts backtracking emits no `call` event, and neither does C-level work. (A quadratic inside a comprehension or generator IS visible — `call` fires once per generator resume.) + WHY THE CLOCK HAD TO GO. The 1.0s bound failed four times across #466 and #474, always on CI's 3.12 job, always between 1.01s and 1.08s, with master re-running green each time. Three local methods disagreed with CI and with each other: uninstrumented ~1% branch-over-parent (inside the parent's own spread), coverage-instrumented indistinguishable, and the whole benchmark file under `--cov` made the branch FASTER. The same harness measured `origin/master` at 94-96ms and again at 89-92ms twenty minutes later. A check that cannot separate a 1% change from a busy runner does not fail safe; it fails expensively. + WHY A BAND AND NOT A CEILING, which is the correction the review forced. A ceiling with headroom is ANOTHER threshold that happens to break — the failure this replaced. The first draft set 470 against a 408 baseline, and measured, that catches nothing smaller than +61 calls on 3.11 and +84 on 3.12: it would have missed the very regression it was built for on four of five interpreters. The cycle's +67 arrived across a dozen PRs at roughly five calls each, and no ceiling loose enough to be safe can see five. A ±2% band trips at +5, verified by injection. A DROP trips it too, which is intended: an unexplained fall is as much a signal as a rise. + PER INTERPRETER, because the count is not machine-independent — 410 on 3.11, 388 on 3.12, 406 on 3.13-3.15 for `parse`. PEP 709 inlined the comprehension frames 3.11 counts and 3.13 added others back. The first draft claimed "the same number on any machine" and took its baseline on 3.11 by luck: had it been taken on 3.12, the 3.11 job would have gone red with nothing in the file explaining why. A version with no row fails loudly with its own number rather than passing unguarded. + WHAT THE OLD BOUND HAD CAUGHT, which is the substantive half. Measured `--against v2.1.0`: `parse` 343 → 410 and `HumanName` 380 → 447, **+67 calls per parse**; by stage, per 1000 parses, `group` 13.5 → 22.2ms, `classify` 11.6 → 13.5, `assign` 11.1 → 13.0, `post_rules` 5.2 → 8.0. The bound was not too tight. The parser had grown into it, and the clock could not say so with enough confidence to act on. + WHERE THE CALLS WENT, per parse, v2.1.0 → here: `_group.py` 94 → 84, `_pieces.py` 0 → 51, `_post_rules.py` 16 → 33, `_vocab.py` 26 → 32, `_assign.py` 21 → 18, `_classify.py` 14 → 16. + AND WHICH PR SPENT THEM, which the first draft got backwards. It named #439's predicate extraction as "the largest contributor" and built a trade-off on it. Measured across its own merge boundary, #439 cost **zero**: 404 → 404, `_group` shedding 51 exactly as `_pieces` gained them. A pure relocation, and `mechanisms.md#ONE-PREDICATE-PER-QUESTION` says as much — the sharing between `group` and `assign` predated the module. The real largest single contributor is **#424 at +23** (381 → 404), the PR that made group's chain and walk mirror assign's; then #404 at +12, with #361, #367 and #434 at +8 each. So the entry's old conclusion — that recovering the cost means reintroducing the drift #439 removed — deterred work on a PR that spent nothing while +23 went unexamined. Recovering it, if ever wanted, starts at #424's peel walks. + ONE REPAIR WAS TRIED AND REVERTED: collapsing `post_rules`' three separate role-index scans into one pass. It saves **6 calls per parse** on 3.11 (the first draft said one, which is not even reachable — `_idx` is a one-line list comprehension, so each removed call frees two frames). Reverted because it was measured against a noisy timing harness that showed nothing; at 6 calls it is 9% of the cycle's growth and worth reconsidering against the band, which can now see it. + RAISING OR LOWERING A ROW IS A DECISION, not a maintenance chore. Append here with the interpreter and the harness invocation, as above. + + ### removed-v1-surface - empty_attribute_default: removed in 2.0 (#255; deprecated in 1.4 per the bridge discipline). Origin #44 (2016): a DB-NULL convenience whose first answer — `name.title or None` — became the migration path. The in-band-signaling bug that sealed it (#254): the 2016 `.replace('None','')` scrub could not tell interpolated None from name text, so "Nonez Smith" rendered diff --git a/docs/release_log.rst b/docs/release_log.rst index 9d589f7f..6b427352 100644 --- a/docs/release_log.rst +++ b/docs/release_log.rst @@ -71,6 +71,7 @@ Release Log - Change case repair to read the parser's own ``conjunction`` tag instead of re-deciding, from the word's spelling, whether a word is a conjunction or an initial. The parse answers that question already -- ``"Scott E. Werner"`` reads ``E.`` as an initial rather than the Italian conjunction -- and the other views honor the answer; case repair asked again, with a shape test applied to each word of a token's text rather than to the token. Two spellings of one name disagreed because of it: ``"juan e-f smith"`` capitalized to ``Juan e-F Smith`` while ``"JUAN E-F SMITH"`` gave ``Juan E-F Smith``; both give ``Juan E-F Smith`` now, ``e-f`` being a middle name and no conjunction of the parse's reading. A conjunction written as a word of its own is untouched, and so is the one-letter carve-out where it applies -- ``"juan y garcia"`` still repairs to ``Juan y Garcia``, ``"JUAN Y GARCIA"`` still to ``Juan Y Garcia``. A field assigned after the parse is unaffected: its text was never classified, so there is no reading to honor and repair asks the vocabulary, applying v1's own predicate the way every earlier version applied it everywhere -- ``h.last = "velasquez y garcia"`` still repairs to ``Velasquez y Garcia`` and ``h.middle = "e."`` to ``E.``. That is the predicate over TODAY's vocabulary, which is narrower than parity with 1.4.0 and the difference is real: ``h.last = "хосе и мария сантос"`` gives ``Хосе И Мария Сантос`` on 1.4.0 and ``Хосе и Мария Сантос`` here, because the Cyrillic ``и`` is a 2.x conjunction and was not a 1.4.0 one. What decides which path a token takes is a mark the assignment leaves, not the absence of a span: a value revised through ``Parser.revise()`` is classified by a sub-parse and keeps its tags, so it repairs as the parse does. One reading does change for hand-built ``Token``\ s in the 2.0 API: an untagged token whose text is conjunction vocabulary is now an ordinary name word and capitalizes, where 2.1 lowercased it -- tags are what the views read, and a hand-built token that carries none is a token with nothing to declare. Case repair is not one of the seven role fields the differential harness compares, so no gate run can see this change either way and none of its counts move; measured directly instead, no name of the 1094-name differential corpus moves under ``capitalized()`` or ``capitalized(force=True)``, its uppercased and lowercased spellings included -- 6564 name/spelling/lexicon rows and 13128 calls (closes #458) + - Change the parse-cost benchmark to bound FUNCTION CALLS per parse rather than wall-clock seconds. The two ``under_a_second`` tests asserted that 1000 parses take under a second; on CI that bound failed four times across two branches at 1.01 to 1.08 seconds while the same code re-ran green on master, and three local measurement methods disagreed with CI and with each other. Frame counts do not move under load, so growth shows up in a diff instead of in a threshold that happens to break. The bound is a per-interpreter band of ±2% -- per interpreter because the count is NOT machine-independent (410 calls for ``parse()`` on 3.11, 388 on 3.12, 406 on 3.13 and later, PEP 709 having inlined the comprehension frames 3.11 counts) -- and a loose five-second backstop stays over both entry points for the class frame counts cannot see, such as a regex that starts backtracking. What the old bound had caught is recorded rather than lost: measured on Python 3.11, 2.2 costs 67 more calls per parse than 2.1.0, of which #424's mirrored peel walks are 23; #439's predicate extraction, which an earlier draft of this note blamed, costs zero. Every figure is recomputable with ``uv run python tools/perf/call_count.py --against v2.1.0``, and the reasoning is in the ``parse-cost`` entry of ``docs/design/decisions.md``; the counts are a dated snapshot, measured 2026-08-31, since nothing in the repository re-runs them. No user-visible behavior changes (closes #475) - Fix a name that opens with a spaced ``Ph. D.`` losing its surname. ``parse("Ph. D. Van Johnson")`` read given ``Van Johnson`` with an empty ``family`` and suffix ``Ph. D.``; it now reads title ``Ph.``, given ``D.``, family ``Van Johnson``. A suffix never begins a name -- position outranks the vocabulary match -- and the merge that heals a split ``Ph.``/``D.`` into one credential is what made a leading credential possible at all: every other suffix-shaped word standing first already falls out as a title (``Jr.``, ``MD``, ``Esq.``, ``Sr.``) or as an ordinary name word (``PhD``, ``III``), so this pair was the only shape that reached the defect. The merge is unchanged everywhere else, and a family comma still opens a listing rather than a name, so ``"John Smith Ph. D."`` keeps suffix ``Ph. D.`` and ``"Smith, Ph. D. Jr."`` keeps suffix ``Ph. D. Jr.``. This RESTORES 1.4.0, whose own healing regex required a preceding space and so could never fire at the head of the string -- measured on the released wheel, three of the four corpus names of this shape return to their v1 reading exactly, and the 1.4.0 ledger loses the rule that used to excuse the difference. The fourth, ``"Ph. D., Jr."``, still differs in where the ``D.`` lands and rides under a pre-comma rule that predates this change. What "the head" means is the head of the STRING, not of the name: a title before the credential keeps it a credential, so ``"Sir Ph. D. Van Johnson"`` still reads given ``Van Johnson`` with no family -- also 1.4.0's reading, and recorded as a boundary in ``rules.md#S2`` rather than left implied. One accepted consequence: ``Parser.revise(suffix="Ph. D.")`` renders ``Ph., D.``, since revise() sub-parses the string it is given and a field value has no head for a head-position rule to read (closes #371) - Fix a trailing surname particle being stranded as a standalone middle name under ``Policy(name_order=FAMILY_FIRST)``, where the same listing written with a comma reads it as part of the surname. A particle ending the name has nothing to link forward to, so what it is doing there is decided by what the writing says: after a family comma it joins the family the comma named and is written before it, and a declared family-first order names the family the same way. ``Parser(policy=Policy(name_order=FAMILY_FIRST)).parse("Jong Anke de")`` gave family ``Jong`` with ``de`` left as a middle name, and now gives family ``de Jong``, given ``Anke`` -- the same answer ``parse("Jong, Anke de")`` has always given. The test is the SLOT the particle landed in, not the word: a middle name is a further given name, which a particle is not, and ``FAMILY_FIRST`` is the only order that puts a trailing piece there. ``FAMILY_FIRST_GIVEN_LAST`` puts it in the given slot, where the caller's own declaration says it is the given name, so ``"Nguyen Thi Van"`` under that order still reads given ``Van``. That one test reads both traditions without asking about the vocabulary at all: ``"Beethoven Ludwig van"`` under ``FAMILY_FIRST`` now gives family ``van Beethoven`` even though ``van`` is one of the 37 particles that are ordinary given names elsewhere. In the same change, a particle standing alone where a family-first order puts the GIVEN name is no longer folded into the family: ``"Ménil de"`` reports given ``de`` under both family-first orders, because that slot holds what the caller declared, and the never-given word list supplies a reading where position leaves the question open rather than overriding one position has already given. Nothing moves under the DEFAULT name order. Measured, 30 of 6606 parses move -- this release's 1101-name corpus under three ``name_order`` values with ``middle_as_family`` off and on -- over twelve names. Those counts cannot come from the differential gate: it parses every corpus name with the default policy and sweeps no policy at all, so no non-default ``name_order`` behavior has ever been compared across versions, and the gate output here is unchanged at all three baselines apart from the corpus names this change's own rules.md examples add. What moves is recomputed by the recipe in the ``P6`` entry of ``docs/design/decisions.md``, which compares the seven role fields against a checkout of the parent commit reading the same corpus files; the count is a dated snapshot, measured 2026-08-30. The ``rules.md#P1`` and ``rules.md#P6`` example lines and ``tests/v2/pipeline/test_post_rules.py`` are what pin the behavior (closes #467) - Fix ``initials()`` reading a name in a different order than the fields of the same name. Two rules fold words into the family name and render them before the rest of it -- ``Policy(middle_as_family=True)``, which sends every middle word to the family, and the tussenvoegsel attachment after a family comma -- and both do it by marking the words rather than moving them, since a parsed word keeps the position it was written at. The ``family`` field reads that mark and ``initials()`` did not, so one parse gave two orders: ``parse("der, y van")`` gave family ``van der`` and initials ``y. d. v.``, and now gives ``y. v. d.``. This RESTORES v1: ``middle_name_as_last`` is v1's spelling of the same option, so most of what moves has a 1.4.0 answer to be measured against, and measured over the 1094-name differential corpus at the default name order, 71 names move under that option, 54 of them back to exactly what 1.4.0 returns and not one of them away from it -- ``"Doe, Dr. John A."`` gives ``J. A. D.`` again where 2.0 through 2.2 gave ``J. D. A.``, and ``"Brundridge, Contessa A"`` gives ``C. A. B.`` where they gave ``C. B. A.``. Of the 17 that match 1.4.0 neither before nor after, 14 now agree with it on the ORDER and differ only in how v1 grouped initials -- 1.4.0 gives one initial per element of its own ``last_list``, so a conjunction-joined surname yields ``V G.`` where the 2.x view, one initial per word, yields ``V. G.``; one more is ``"der, y van"``, whose family is nothing but particles, where 1.4.0 contributes no initial at all and 2.x contributes its words, a difference this release does not touch; and the remaining two parse differently from 1.4.0 for reasons that predate this fix. Without the option, one corpus name moves, the ``"der, y van"`` above -- and it is one of one, since it is the only name in the corpus whose family holds two contributing words with a folded one behind the other, the shape an order change can be seen in at all. ``HumanName.initials()`` was already right and is unchanged, measured: no name of the 1094-name corpus moves through the facade, with the option or without it. It reads ``first_list``/``middle_list``/``last_list``, which prepend the folded words as v1 did, so it is the 2.0 API's ``ParsedName.initials()`` that was out of step -- with the field beside it, with the facade, and with 1.4.0 at once. ``initials()`` is not one of the seven role fields the differential harness compares, so no gate run can see this change: run at all three baselines before and after, the output is identical to the byte -- 1094 corpus names, 229 / 194 / 102 intentional diffs and ``unexplained: 0`` at 1.4.0 / 2.0.0 / 2.1.0, with every per-heading count unchanged. So where the counts in this bullet come from has to be said, the gate's classified summary not being able to supply them and the ``rules.md#R3`` example line witnessing the order without counting anything: what MOVES is recomputed by the recipe in the ``R3`` entry of ``docs/design/decisions.md``, which compares this view against the pre-change rendering over these same four corpora and reproduces the 71 and the one-of-one; the 1.4.0 comparisons and the facade sweep are dated snapshots rather than re-derivable ones, measured 2026-08-30 against the released 1.4.0 wheel and against the pre-change tree, which nothing in the repository re-runs. The ``rules.md#R3`` example line and ``tests/v2/test_render.py`` are what pin the behavior (closes #408) diff --git a/tests/v2/test_benchmark.py b/tests/v2/test_benchmark.py index a2cb5b3f..02d6b388 100644 --- a/tests/v2/test_benchmark.py +++ b/tests/v2/test_benchmark.py @@ -19,6 +19,7 @@ leaves empty. A shape guards nothing if the default policy cannot reach the code under it. """ +import sys import time from collections.abc import Callable @@ -28,26 +29,154 @@ from nameparser._policy import Policy -def test_parse_thousand_names_under_a_second() -> None: - parse("warm up the default parser cache") - start = time.perf_counter() - for i in range(1000): - parse(f"Dr. Juan{i} de la Vega III") - elapsed = time.perf_counter() - start - assert elapsed < 1.0, f"1000 parses took {elapsed:.2f}s" +#: What one parse of the reference name is allowed to cost, counted in +#: Python frame entries rather than seconds (#475). A wall-clock bound +#: cannot separate a 1% change from a busy CI runner: the 1.0s version +#: of these tests failed four times across #466 and #474 at 1.01-1.08s +#: while master re-ran green each time, and three local measurement +#: methods disagreed with CI and with each other. Frame counts do not +#: move under load, and -- measured -- do not move under `pytest --cov` +#: either, though coverage costs 3x on the clock. +#: +#: PER INTERPRETER, because the count is not machine-independent: PEP +#: 709 inlined the comprehension frames 3.11 counts, and 3.13 added +#: others back. A version with no row here fails loudly with its own +#: number rather than passing unguarded -- recording it is the point. +#: +#: A BAND, not a ceiling. A ceiling with headroom is another threshold +#: that happens to break, which is the failure this replaced: the 2.2 +#: cycle's +67 calls arrived across a dozen PRs at roughly five each, +#: and no ceiling loose enough to be safe can see five. The band is +#: +/-2%, so a single PR's worth of growth lands in a diff with a +#: reason beside it. A DROP is a signal too, and trips the same test. +#: +#: Raising or lowering a row is a decision to record in +#: decisions.md#parse-cost, not a maintenance chore. Recompute with +#: `uv run python tools/perf/call_count.py`, which is the harness every +#: number in that entry comes from. +#: +#: Measured 2026-08-31 on this tree with that harness. 3.11 and 3.14 +#: are the two interpreters on the author's machine; 3.12, 3.13 and +#: 3.15 are CI's, seeded from a review measurement and confirmed by +#: the first green run -- a wrong seed fails with the real number. +_CALL_BASELINE = { + (3, 11): {"parse": 410, "facade": 447}, + (3, 12): {"parse": 388, "facade": 425}, + (3, 13): {"parse": 406, "facade": 443}, + (3, 14): {"parse": 406, "facade": 443}, + (3, 15): {"parse": 406, "facade": 443}, +} +_BAND = 0.02 + +#: The reference name is fixed WIDTH, not merely fixed: an incrementing +#: counter grows a digit and costs one more call when it does, which +#: made the first draft's "mean over n names" a function of n rather +#: than of the parser. +_REFERENCE = "Dr. Juan{i:04d} de la Vega III" + + +def _calls_per_parse(fn: Callable[[str], object], n: int = 50) -> float: + """Mean Python frame entries for one parse of the reference name. + + `sys.setprofile` counts Python frame entries -- NOT C calls, and not + the interpreter's own work inside one frame -- so this measures the + work the parser does rather than how fast the machine did it. Per + parse there are roughly 575 `c_call` events this cannot see. + + Deterministic for a given tree AND interpreter: verified identical + across repeated calls, fresh processes, PYTHONHASHSEED values, and + with or without coverage installed. + """ + fn("warm up the caches") + calls = 0 + + def counter(frame: object, event: str, arg: object) -> None: + nonlocal calls + if event == "call": + calls += 1 + sys.setprofile(counter) + try: + for i in range(n): + fn(_REFERENCE.format(i=i)) + finally: + sys.setprofile(None) + return calls / n -def test_facade_thousand_names_under_a_second() -> None: + +def _check_budget(kind: str, fn: Callable[[str], object]) -> None: + """Assert one entry point sits inside its band. + + Skips rather than clobbers when something else owns the profile + slot: `sys.setprofile(None)` in the helper CLEARS the hook, and a + maintainer profiling the parser -- the very workflow that produced + decisions.md#parse-cost -- would otherwise get silently truncated + data. Restoring is not an option: `sys.getprofile()` hands back a + `Profile` object that `sys.setprofile()` then refuses. + """ + if sys.getprofile() is not None: + pytest.skip("a profile hook is already installed; this test owns it") + version = sys.version_info[:2] + if version not in _CALL_BASELINE: + actual = _calls_per_parse(fn) + pytest.fail( + f"no call baseline for Python {version[0]}.{version[1]}; " + f"{kind} measures {actual:.1f} here. Add the row to " + f"_CALL_BASELINE and record it in decisions.md#parse-cost") + baseline = _CALL_BASELINE[version][kind] + actual = _calls_per_parse(fn) + low, high = baseline * (1 - _BAND), baseline * (1 + _BAND) + assert low <= actual <= high, ( + f"{kind} costs {actual:.1f} calls/name on Python " + f"{version[0]}.{version[1]}, band {low:.0f}-{high:.0f} around a " + f"baseline of {baseline}. Growth and shrinkage are both signals: " + f"recompute with tools/perf/call_count.py, then either make it " + f"cheaper or move the baseline deliberately -- see " + f"decisions.md#parse-cost") + + +def test_parse_cost_stays_within_its_band() -> None: + _check_budget("parse", parse) + + +def test_facade_cost_stays_within_its_band() -> None: # the legacy-API path (what all existing users call): snapshot - # resolution must stay generation-cached, not rebuilt per instance + # resolution must stay generation-cached, not rebuilt per instance. + # Measured: defeating that cache costs 4826 calls against this + # band, while taking only 0.93s per 1000 -- which the 1.0s + # wall-clock test this replaced would have PASSED. from nameparser import HumanName - HumanName("warm up the caches") + _check_budget("facade", HumanName) + + +@pytest.mark.parametrize("kind,fn", [ + ("parse", parse), + ("facade", lambda name: __import__("nameparser").HumanName(name)), +]) +def test_a_thousand_names_still_parse_in_reasonable_time( + kind: str, fn: Callable[[str], object]) -> None: + """The order-of-magnitude backstop the call bands do not give. + + Frame counts cannot see work that happens without entering a Python + frame: a compiled regex that starts backtracking emits no `call` + event at all, and neither does a C-level structure turning + quadratic. (A quadratic inside a comprehension or generator IS + visible -- `call` fires once per generator resume.) Both entry + points are covered, because both failures that motivated #475 were + on the facade and the first draft of this replacement guarded only + `parse`. + + The bound is loose enough that runner variance cannot reach it: the + failures were at 1.01-1.08s against 1.0, and coverage costs about + 3x, so 5s leaves roughly 5x of margin on the CI runner that failed. + """ + fn("warm up the caches") start = time.perf_counter() for i in range(1000): - HumanName(f"Dr. Juan{i} de la Vega III") + fn(_REFERENCE.format(i=i)) elapsed = time.perf_counter() - start - assert elapsed < 1.0, f"1000 facade parses took {elapsed:.2f}s" + assert elapsed < 5.0, f"1000 {kind} parses took {elapsed:.2f}s" # Pathological shapes: each repeats a unit that drives one stage's inner diff --git a/tools/perf/call_count.py b/tools/perf/call_count.py new file mode 100644 index 00000000..b7270bea --- /dev/null +++ b/tools/perf/call_count.py @@ -0,0 +1,157 @@ +"""Count Python function calls for one parse, as tests/v2/test_benchmark.py +counts them. + +WHY THIS IS IN THE TREE. #475 replaced a wall-clock benchmark with a +call-count budget, and the decision entry that justifies it quotes a +dozen numbers -- per interpreter, per module, per commit. The first +draft of that entry took them across several sessions with throwaway +scripts, on whichever interpreter was to hand, and published a table +that spliced one interpreter's "before" column onto another's "after". +Four of its eight module figures were wrong and its four stage figures +were off by 2x, none of it visible without the harness. + +docs/design/AGENTS.md already asks for this: "Where an entry quotes +something that drifts, give the one-liner that recomputes it." This is +that one-liner. Every number in decisions.md#parse-cost comes from a +mode of this script, named beside it. + + uv run python tools/perf/call_count.py # both entry points + uv run python tools/perf/call_count.py --modules # calls by module + uv run python tools/perf/call_count.py --stages # ms per 1000, per stage + uv run python tools/perf/call_count.py --against REF # this tree vs a git ref + +The interpreter is printed with every result and belongs beside any +number quoted from it: the count is deterministic for a given +(tree, interpreter), not across interpreters. Measured 2026-08-31, +one parse of the reference name: 407 on 3.11, 385 on 3.12, 403 on +3.13/3.14/3.15 -- PEP 709 inlined the comprehension frames 3.11 +counts, and 3.13 added others back. +""" +from __future__ import annotations + +import argparse +import subprocess +import sys +import tempfile +import time +from collections.abc import Callable +from pathlib import Path + +#: The name every figure is quoted for. Fixed width on purpose: an +#: incrementing counter grows a digit and adds one call as it does, so +#: a mean over N names is a function of N rather than of the parser. +REFERENCE = "Dr. Juan{i:04d} de la Vega III" + + +def calls_for(fn: Callable[[str], object], n: int = 50) -> float: + """Mean Python frame entries for one parse of the reference name.""" + fn("warm up the caches") + calls = 0 + + def counter(frame: object, event: str, arg: object) -> None: + nonlocal calls + if event == "call": + calls += 1 + + sys.setprofile(counter) + try: + for i in range(n): + fn(REFERENCE.format(i=i)) + finally: + sys.setprofile(None) + return calls / n + + +def by_module(fn: Callable[[str], object], n: int = 500) -> dict[str, float]: + """Calls per parse attributed to each nameparser module.""" + import cProfile + import pstats + + fn("warm up the caches") + pr = cProfile.Profile() + pr.enable() + for i in range(n): + fn(REFERENCE.format(i=i)) + pr.disable() + out: dict[str, float] = {} + for (path, _, _), stat in pstats.Stats(pr).stats.items(): + if "nameparser" in path: + mod = path.split("nameparser/")[-1] + out[mod] = out.get(mod, 0) + stat[0] / n + return out + + +def by_stage(n: int = 1000) -> dict[str, float]: + """Milliseconds per 1000 parses, per pipeline stage. + + Wall-clock, so unlike the counts above this drifts with the + machine; quote it as a dated snapshot or not at all. + """ + from nameparser import Lexicon, Policy + from nameparser._pipeline import STAGES + from nameparser._pipeline._state import ParseState + + lex, pol = Lexicon.default(), Policy() + names = [REFERENCE.format(i=i) for i in range(n)] + for name in names[:50]: + state = ParseState(original=name, policy=pol, lexicon=lex) + for stage in STAGES: + state = stage(state) + total: dict[str, float] = {} + for name in names: + state = ParseState(original=name, policy=pol, lexicon=lex) + for stage in STAGES: + start = time.perf_counter() + state = stage(state) + total[stage.__name__] = (total.get(stage.__name__, 0) + + time.perf_counter() - start) + return {k: v * 1000 * (1000 / n) for k, v in total.items()} + + +def _in_ref(ref: str, mode: str) -> str: + """Run this script's own measurement inside a checkout of `ref`.""" + with tempfile.TemporaryDirectory() as tmp: + subprocess.run(f"git archive {ref} | tar -x -C {tmp}", + shell=True, check=True) + me = Path(tmp) / "tools" / "perf" / "call_count.py" + if not me.exists(): # ref predates this script + me.parent.mkdir(parents=True, exist_ok=True) + me.write_text(Path(__file__).read_text(encoding="utf-8"), + encoding="utf-8") + done = subprocess.run( + [sys.executable, str(me)] + ([mode] if mode else []), + cwd=tmp, capture_output=True, text=True) + return done.stdout.strip() or done.stderr.strip() + + +def main() -> None: + ap = argparse.ArgumentParser(description=__doc__) + ap.add_argument("--modules", action="store_true") + ap.add_argument("--stages", action="store_true") + ap.add_argument("--against", metavar="REF", + help="also measure a git ref, in its own checkout") + args = ap.parse_args() + version = f"{sys.version_info.major}.{sys.version_info.minor}" + + sys.path.insert(0, str(Path.cwd())) + from nameparser import HumanName, parse + + if args.modules: + for mod, count in sorted(by_module(HumanName).items(), + key=lambda kv: -kv[1]): + print(f"py{version} {count:7.1f} {mod}") + elif args.stages: + for stage, ms in sorted(by_stage().items(), key=lambda kv: -kv[1]): + print(f"py{version} {ms:7.1f}ms per 1000 {stage}") + else: + print(f"py{version} parse={calls_for(parse):.2f} " + f"facade={calls_for(HumanName):.2f}") + if args.against: + mode = ("--modules" if args.modules + else "--stages" if args.stages else "") + print(f"--- {args.against} ---") + print(_in_ref(args.against, mode)) + + +if __name__ == "__main__": + main()