Skip to content

perf: Reduce best fitting allocations#7411

Merged
MichaReiser merged 2 commits intomainfrom
best-fitting-reduce-allocations
Sep 18, 2023
Merged

perf: Reduce best fitting allocations#7411
MichaReiser merged 2 commits intomainfrom
best-fitting-reduce-allocations

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Sep 15, 2023

Let's try this again, now that we have more best fitting elements.

Summary

This PR removes the number of allocations necessary when using BestFitting.

Best fitting performs the following allocations today:

  1. It allocates one Vec in which it stores all variants
  2. For each variant, allocate a Vec<FormatElement> that stores the variant's IR elements

This PR eliminates 2. It uses StartBestFittingEntry and EndBestFittingEntry markers to identify the start/end of a variant. The outer vector is still required because it otherwise gets fairly complicated to identify the start/end of a variant in case we encounter nested best fittings.

This won't have as much impact anymore when #7475 lands but we'll need best fitting to implement Black's preview string processing

Test Plan

cargo test

@codspeed-hq
Copy link

codspeed-hq bot commented Sep 15, 2023

CodSpeed Performance Report

Merging #7411 will improve performances by 7.76%

Comparing best-fitting-reduce-allocations (441a75f) with main (2421805)

Summary

⚡ 4 improvements
✅ 21 untouched benchmarks

Benchmarks breakdown

Benchmark main best-fitting-reduce-allocations Change
formatter[unicode/pypinyin.py] 3.8 ms 3.6 ms +3.21%
formatter[large/dataset.py] 61.7 ms 57.3 ms +7.76%
formatter[pydantic/types.py] 21.4 ms 20.8 ms +3.18%
formatter[numpy/ctypeslib.py] 11.1 ms 10.7 ms +3.64%

@MichaReiser MichaReiser force-pushed the best-fitting-reduce-allocations branch from 1a0f9f8 to a43c245 Compare September 16, 2023 17:53
@MichaReiser
Copy link
Member Author

MichaReiser commented Sep 16, 2023

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@MichaReiser MichaReiser force-pushed the best-fitting-reduce-allocations branch from a43c245 to cc50546 Compare September 16, 2023 18:08
@MichaReiser MichaReiser added performance Potential performance improvement formatter Related to the formatter labels Sep 16, 2023
@MichaReiser MichaReiser marked this pull request as ready for review September 16, 2023 18:25
@MichaReiser MichaReiser force-pushed the best-fitting-reduce-allocations branch from cc50546 to ddd5e34 Compare September 16, 2023 18:53
@MichaReiser MichaReiser force-pushed the best-fitting-reduce-allocations branch from ddd5e34 to 2843fc0 Compare September 17, 2023 13:58
@MichaReiser MichaReiser enabled auto-merge (squash) September 18, 2023 19:42
@MichaReiser MichaReiser merged commit 3336d23 into main Sep 18, 2023
@MichaReiser MichaReiser deleted the best-fitting-reduce-allocations branch September 18, 2023 19:49
Boshen added a commit to oxc-project/oxc that referenced this pull request Jan 16, 2026
Port of astral-sh/ruff#7411 to oxc_formatter.

This optimizes memory allocation for `BestFitting` elements by using a flat
storage structure with marker tags instead of nested allocations.

Key changes:
- Add `StartBestFittingEntry`/`EndBestFittingEntry` tags to delimit variants
- Change `BestFittingElement` from `&[&[FormatElement]]` to `&[FormatElement]`
- Add `BestFittingVariantsIter` iterator to parse variants from flat structure
- Update printer to handle the new structure

Benefits:
- Reduces memory allocations by avoiding per-variant heap allocations
- All variant content stored in a single contiguous buffer

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

formatter Related to the formatter performance Potential performance improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants