perf: reuse configuration alias traversal results#124
Conversation
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
Closes #123
Reproduction and root cause
A size-N non-empty acyclic payload shared by N override keys was traversed once per alias before the clone memo was consulted. The original probe recorded 130, 460, 1,720, and 6,640 element visits for N=10, 20, 40, and 80.
The first candidate cached scans only until a nested mapping merge changed the active projection map. Adversarial review reproduced the same quadratic work when payload aliases were interleaved with sibling mapping merges. The corrected implementation owns the cache per stack frame: a child receives a fresh cache, while its parent resumes with the cache valid for the restored parent projection.
The bounded after probe records 40, 80, 160, and 320 visits for N=10, 20, 40, and 80. The interleaved sibling-merge regression remains linear as well.
Scope and non-goals
This changes only deep-merge replacement-closure reuse for non-empty acyclic shared payloads. Existing identity, source isolation, recursive/path-local projection, tuple/list cycle, compact mapping DAG, and deep iterative semantics remain covered. It does not redesign the graph cloner or add a new performance guarantee for cyclic root-backreference graphs.
Verification
uv sync --extra dev --extra docsuv run gobbler --version→ 0.2.32uv run pytest tests/unit/test_config.py -q→ 58 passeduv run pytest tests/unit/ -q --tb=short→ 768 passeduv run pytest tests/integration/ -q -m integration --tb=short→ 2 passeduv run ruff format --check src/ tests/uv run ruff check src/ tests/uv run --extra docs mkdocs build --strictgit diff --checkReview
Round one primary and CI/release reviews passed. Graph review found the interleaved-sibling invalidation case; the single corrective edit added per-frame cache ownership and a regression. Round two primary and CI/release reviews passed. Round two graph review raised cyclic root-backreference complexity; independent adjudication classified it out of scope because #123's performance contract is explicitly acyclic while cyclic semantics remain preserved.
Risk and documentation
Risk is limited to cache lifetime in the iterative configuration merge. Each cache is scoped to exactly one stack frame and therefore one stable active-projection set. Both changelogs, package/CLI/queue versions, browser-extension metadata, and static version assertions are synchronized to 0.2.32.