-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Avoid allocating in lex_decimal #7252
Conversation
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
PR Check ResultsEcosystem✅ ecosystem check detected no changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I like the LexedText
abstraction. I think it could even be used in f-string normalization to normalize {{
/}}
to {
/}
using LexedText::skip_char
.
@@ -1236,6 +1220,49 @@ const fn is_python_whitespace(c: char) -> bool { | |||
) | |||
} | |||
|
|||
enum LexedText<'a> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is that the same abstraction we use for normalizing strings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lexer doesn't normalize Strings. I haven't looked into what the whole string.rs
is doing (and how much of it could be moved into the lexer)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i meant in the formatter, this was just a random thought because the logic looked familiar from the normalize functions of strings, ints and floats
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's similar but we don't use an abstraction in the formatter.
Summary
lex_decimal_number
shows up as one of the most expensive operations during lexing. This is partially due to it allocating a string before calling intof64::parse
orBigInt::parse
.This PR avoids allocating a string for numbers that don't use
_
orE
.Test Plan
cargo test
This improves performance for files with many numbers (dataset.py +3%)