Repository navigation
Improve formatting performance for subscript chains - #5275
Closed
happykawayigt wants to merge 2 commits into
Closed
happykawayigt wants to merge 2 commits into
happykawayigt wants to merge 2 commits into
Conversation
happykawayigt
marked this pull request as ready for review
August 1, 2026 18:24
cobaltt7
reviewed
Oct 3, 2026
| with patch.object(BracketTracker, "mark", counting_mark): | ||
| formatted = black.format_str(source, mode=Mode()) | ||
|
|
||
| assert len(formatted.splitlines()) == 233 |
Collaborator
There was a problem hiding this comment.
This exact number could get confusing to update if formatting changes.
Not sure that it even makes sense to check the formatting, the test is of the performance, not of the formatting.
Collaborator
|
Could you add comments throughout this code? This is a fairly large code change and I find it difficult to follow everywhere - particularly the changes inside |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #5270.
Closes #5475
Closes #5305
Long chains of single-token subscript trailers repeatedly rebuild the same prefix
LineandBracketTrackerstate during recursive right-hand splitting. This patch detects that narrowly defined no-comment shape, reuses the already validated leaf metadata, caches cumulative ASCII widths for prefix heads, and falls back to the existing path for complex subscripts, comments, non-ASCII text, or other bracket shapes.The formatting result is unchanged. In local CPython 3.12 benchmarks for
x = a[0][1]...at the default line length:Output SHA-256 matched the base branch at every benchmark size. A 135-case matrix covering numeric, name, and string subscripts across multiple line lengths and contexts also matched byte-for-byte.
Validation:
pytest tests/test_format.py -q(230 passed)pytest tests/test_black.py -q -k "not symlink"(160 passed, 6 deselected, 8 subtests passed)git diff --checkBracketTracker.mark()calls and verifies idempotenceThe three excluded
test_black.pycases require Windows symlink privileges and fail locally withWinError 1314; they are unrelated to this change.AI assistance was used during implementation. I independently reproduced the issue, profiled the split path, reviewed the final diff, compared outputs against an untouched base worktree, and ran the validation above.
Checklist - did you ...
--previewstyle, following the stability policy? (No style change.)CHANGES.mdif necessary?