Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cold attribute to less likely printer queue branches #10121

Merged
merged 2 commits into from
Feb 26, 2024

Conversation

MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Feb 25, 2024

Summary

The formatter writes most elements to a single Vec<FormatElement> except for:

  • Memoized content: It's sometimes necessary to emit the same content but using two different layouts. The formatter memoizes/interns the content's IR to avoid formatting it twice. The internet content stores the elements in its own Vec
  • BestFitting layout: The BestFitting IR allows multiple different layouts for the same content. It stores the IR elements of the variants in a dedicated Vec

The fact that the Printer must support multiple Vecs add complexity to its queue because the queue needs to support a stack of slices where it pops elements from the top slice but takes the next slice if the top slice is empty.

This PR adds a few #[cold] attributes signaling that it's unlikely that the top slice is empty because a) the printer takes elements from the main top-level Vec most of the time and b) most slices contain many elements, but have only one end ;)

This gives us a solid 2-3% performance improvement in the printer (I no longer see FitsQueue::pop in the top methods when profiling the large dataset)

image

@BurntSushi won't believe this

Test

cargo test

@MichaReiser MichaReiser added performance Potential performance improvement formatter Related to the formatter labels Feb 25, 2024
Copy link
Contributor

ruff-ecosystem results

Formatter (stable)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

openai/openai-cookbook (error)

warning: Detected debug build without --no-cache.
error: Failed to read examples/How_to_handle_rate_limits.ipynb: Expected a Jupyter Notebook, which must be internally stored as JSON, but this file isn't valid JSON: trailing comma at line 47 column 4

Formatter (preview)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

openai/openai-cookbook (error)

ruff format --preview

warning: Detected debug build without --no-cache.
error: Failed to read examples/How_to_handle_rate_limits.ipynb: Expected a Jupyter Notebook, which must be internally stored as JSON, but this file isn't valid JSON: trailing comma at line 47 column 4

@MichaReiser MichaReiser marked this pull request as ready for review February 25, 2024 20:19
Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's neat.

Copy link
Member

@BurntSushi BurntSushi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow. Rather impressive. It's very rare to see #[cold] matter. 🎉 Nice find!

@MichaReiser MichaReiser merged commit 761d4d4 into main Feb 26, 2024
17 checks passed
@MichaReiser MichaReiser deleted the printer-queue-experiment-with-cold branch February 26, 2024 07:19
nkxxll pushed a commit to nkxxll/ruff that referenced this pull request Mar 10, 2024
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.

3 participants