Skip to content

Speed up key lookups and replacements in tables - #704

Open
DicksonWu654 wants to merge 1 commit into
python-poetry:masterfrom
DicksonWu654:perf/key-lookups
Open

DicksonWu654 wants to merge 1 commit into
python-poetry:masterfrom
DicksonWu654:perf/key-lookups

Conversation

@DicksonWu654

Copy link
Copy Markdown

Summary

Two costs in Container grow with the size of the table rather than with the key being accessed:

  • Container.__setitem__ on an existing key ran next(filter(lambda k: k == key, self._map)) to recover the stored Key object, comparing the query against every key in the table. Updating every key of a table therefore takes quadratic time. _map already points to the key's body slot, and that slot holds the stored key, so take it from there.
  • item() and __contains__ turned every str query into a SingleKey (which runs key-type detection and builds the quoted form) just to probe _map. SingleKey hashes and compares like its string, so a plain str can probe _map directly. On a miss, NonExistentKey is still built from a SingleKey, so error messages are unchanged.

A container stores only SingleKeys unless user code passes in another Key implementation; dotted keys are split into SingleKeys before they are stored. A boolean flag records whether any other Key type was ever stored. If so, the container keeps the old normalization and comparison scan, because such a key's __eq__ might not accept a str. The flag is carried through copy, deepcopy and unpickling. Its only purpose is to keep user-defined Key subclasses behaving exactly as before. It could be dropped if you don't think that case needs preserving.

There is no other new state, and no change to the public API or pickle format.

Benchmarks

Master is 55d0768. Each pair is two fresh processes pinned to one core with taskset, with the master-first / branch-first order alternating. Each process warms up first (plus 2 s on PyPy) and reports its fastest call. The table shows the median of those times and the median of the per-pair ratios, with their range. The machine is a shared, busy 11-vCPU Linux box (WSL2), so the ranges are wide.

Workload Python master branch speedup (range) pairs
2,000 shuffled edits to one table 3.12 1793 ms 277 ms 7.41x (3.43–19.6) 7
parse + those edits + dump 3.12 1844 ms 278 ms 6.62x (3.57–11.1) 7
2,000 shuffled edits to one table 3.14 668 ms 78.6 ms 8.50x (6.05–10.7) 5
2,000 shuffled edits to one table PyPy 3.11 239 ms 35.7 ms 6.99x (6.03–9.15) 5
read name/version of every package, uv.lock 3.12 0.639 ms 0.189 ms 3.28x (2.82–4.86) 5
read name/version of every package, poetry.lock 3.12 1.19 ms 0.342 ms 3.74x (2.29–6.73) 7
read name/version of every package, uv.lock 3.14 0.639 ms 0.130 ms 4.49x (4.01–6.29) 5
parse uv.lock 3.12 385 ms 354 ms 1.11x (0.80–1.37) 11
parse poetry.lock 3.12 142 ms 170 ms 0.95x (0.62–1.39) 11
parse uv.lock 3.14 343 ms 367 ms 0.97x (0.54–1.11) 9
parse + set every package version + dump, uv.lock 3.12 355 ms 365 ms 0.97x (0.94–1.17) 5

The two 3.12 rows for 2,000 edits, and the poetry.lock lookup row, average each process over at least 0.4 s instead of taking its fastest call.

  • 2,000 shuffled edits: a [tool.poetry.dependencies] table with 2,000 entries, with a new version string assigned to every key in random order.
  • Lock-file reads: real uv.lock / poetry.lock files.

Parse times, and edits to lock files whose tables have only a few keys, show no measurable change (the bottom rows).

Behaviour checks

  • The full suite, including the toml-test submodule, passes on CPython 3.9, 3.12 and 3.14, and on PyPy 3.11. ruff check, ruff format --check and pyupgrade --py39-plus are clean. mypy reports the same errors as on master.
  • New tests in tests/test_container_updates.py cover:
    • the stored key's quoting and trivia surviving repeated updates through str, str subclasses and Key queries,
    • updates after deletions and on out-of-order tables,
    • missing-key error messages,
    • no comparisons against unrelated keys,
    • custom Key subclasses through copy, deepcopy and pickle.
  • A differential check against master compared rendered text, item types and trivia, unwrap() results, edit-then-dump output, and exact exception messages. It covered every TOML file in tests/toml-test and tests/examples plus real lock files. Earlier, roughly 17,000 generated documents and edit sequences on CPython 3.12 and 3.14 were also compared. No differences were found.

Duplicate Check

Agent Drafting Metadata

  • Agent: OpenAI Codex (implementation, tests, benchmarks); Claude Code (independent re-check and PR preparation)
  • Model: GPT-6; Claude Opus 5.5
  • Notes: Before submission, the change was re-checked independently against master. That re-check reran the full test suite on four interpreters, ran a separate differential check, and repeated the paired benchmarks above.

🤖 Generated with Claude Code

Replacing an existing key scanned every key in the container to recover
the stored Key object, so updating many keys of a large table was
quadratic. Take the stored key from the body slot that _map already
points to instead.

Looking up a plain str key built and validated a temporary SingleKey
just to probe _map. SingleKey hashes and compares like its string, so
probe _map with the string directly. Containers that have ever held a
custom Key implementation keep the previous normalization and
comparison scan, since such keys may not accept a str operand. Missing
key errors still report the normalized key.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@human-attestation

Copy link
Copy Markdown

🛡️ Human Attestation required. A human needs to confirm this PR once before it can merge.

Attest this PR — sign in with GitHub, pass the challenge, and confirm one of the statements.

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