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

Reduce Result<Tok, LexicalError> size by using Box<str> instead of String #9885

Merged
merged 3 commits into from
Feb 8, 2024

Conversation

MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Feb 7, 2024

  • Reduce the LexicalError type by using Box<str> over String
  • Reduce the Tok size by using Box<str> over String.

Using Box<str> is sufficient for us because we don't want to mutate the error message or token value.

The downside of this is that using Box<str> is slightly more annoying.

Future: It would be interesting to change the Lexer signature always to return a Tok::Invalid and write the errors to a Vec instead. This should allow for drastically reducing the size of Tok and Spanned.

@MichaReiser MichaReiser changed the base branch from main to lexer-perf February 7, 2024 22:16
Copy link

codspeed-hq bot commented Feb 7, 2024

CodSpeed Performance Report

Merging #9885 will not alter performance

Comparing reduce-tok-size (7a7f01c) with main (9027169)

Summary

✅ 30 untouched benchmarks

Copy link
Contributor

github-actions bot commented Feb 7, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@MichaReiser MichaReiser force-pushed the reduce-tok-size branch 3 times, most recently from 4c7142b to e232f73 Compare February 8, 2024 03:07
@MichaReiser MichaReiser added the performance Potential performance improvement label Feb 8, 2024
@MichaReiser MichaReiser changed the title Reduce Tok size by using Box<str> instead of String Reduce Tok and Result<Tok, LexicalError> sizes by using Box<str> instead of String Feb 8, 2024
@MichaReiser MichaReiser changed the title Reduce Tok and Result<Tok, LexicalError> sizes by using Box<str> instead of String Reduce Result<Tok, LexicalError> size by using Box<str> instead of String Feb 8, 2024
@MichaReiser MichaReiser changed the base branch from lexer-perf to main February 8, 2024 12:04
@MichaReiser MichaReiser closed this Feb 8, 2024
@MichaReiser MichaReiser reopened this Feb 8, 2024
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.

Nice!!!

@charliermarsh
Copy link
Member

I'm not kidding I was thinking about this as I fell asleep a few nights ago.

@MichaReiser MichaReiser marked this pull request as ready for review February 8, 2024 18:54
@MichaReiser MichaReiser merged commit fe7d965 into main Feb 8, 2024
17 checks passed
@MichaReiser MichaReiser deleted the reduce-tok-size branch February 8, 2024 20:36
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
performance Potential performance improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants