Repository navigation
Specialize ASCII identifier lexing - #29503
charliermarsh merged 2 commits into
Conversation
GitHub Actions
Inactive
Memory usage reportMemory usage unchanged ✅ |
|
|
Merging this PR will improve performance by 8.08%1 warning
Performance Changes
Tip Curious why performance improved? Comment Comparing 
Footnotes
|
dee63d0 to
9d1d502
Compare
GitHub Actions
Inactive
MichaReiser
left a comment
MichaReiser
left a comment
There was a problem hiding this comment.
I don't know why but GitHub does not show me the approve button 😆 but consider this approved
| self.state = State::Other; | ||
|
|
||
| identifier | ||
| self.lex_unicode_identifier() |
There was a problem hiding this comment.
Is it an issue that we now assign the state before lexing the identifier?
|
|
||
| /// Finishes an identifier after consuming a non-ASCII identifier character. | ||
| fn lex_unicode_identifier(&mut self) -> TokenKind { | ||
| let ascii_len = ascii_identifier_prefix_len(self.cursor.rest().as_bytes()); |
There was a problem hiding this comment.
Is skipping ascii caracters after a non-ASCII character worth it? Is the assumption here that a non-ASCII character is commonly followed by a long ASCII only sequence. I'm not sure this holds true and is also not something we do for unicode characters in the middle of a word.
| if self.cursor.eat_if(is_xid_continue).is_some() { | ||
| return self.lex_unicode_identifier(); | ||
| } |
There was a problem hiding this comment.
Could we avoid starting over if there's a non ascii character? Seems a bit wasteful to throw away what we already lexed.
| self.current_flags |= TokenFlags::NON_ASCII_IDENTIFIER; | ||
| return TokenKind::Identifier; | ||
| match bytes.get(ascii_len) { | ||
| Some(quote) if ascii_len <= 1 && matches!(quote, b'\'' | b'"') => { |
There was a problem hiding this comment.
| Some(quote) if ascii_len <= 1 && matches!(quote, b'\'' | b'"') => { | |
| Some(b'\'' | b'"') if ascii_len <= 1 => { |
GitHub Actions
Inactive
9d1d502 to
a52df1b
Compare
GitHub Actions
Inactive
GitHub Actions
Inactive
7460521 to
cc8580c
Compare
GitHub Actions
Inactive
Memory usage reportMemory usage unchanged ✅ Results for cc8580c · Workflow run |
|
|
ntBre
added
performance
Summary
We give ASCII identifiers a dedicated path, avoiding Unicode bookkeeping and checking string prefixes only when a one- or two-letter name reaches a quote. Both paths share the existing batched ASCII scanner, preserving fast scanning for Unicode names with long ASCII suffixes.
Stacked on #29463 to measure the incremental improvement. CodSpeed reports 6.6–9.8% greater efficiency across all five lexer benchmarks, with no reported regressions. We preserve the existing scanner and add no inlining attributes or unsafe code.