Skip to content

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

Merged
MichaReiser merged 3 commits into
mainfrom
reduce-tok-size
Feb 8, 2024
Merged

Reduce Result<Tok, LexicalError> size by using Box<str> instead of String#9885
MichaReiser merged 3 commits into
mainfrom
reduce-tok-size

Conversation

@MichaReiser

@MichaReiser MichaReiser commented Feb 7, 2024

Copy link
Copy Markdown
Member
  • 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
@codspeed-hq

codspeed-hq Bot commented Feb 7, 2024

Copy link
Copy Markdown

CodSpeed Performance Report

Merging #9885 will not alter performance

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

Summary

✅ 30 untouched benchmarks

@github-actions

github-actions Bot commented Feb 7, 2024

Copy link
Copy Markdown
Contributor

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

@BurntSushi BurntSushi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice!!!

Comment thread crates/ruff_linter/src/rules/pycodestyle/rules/invalid_escape_sequence.rs Outdated
@charliermarsh

Copy link
Copy Markdown
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
@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