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

perf: optimize identifier parsing #295

Merged
merged 1 commit into from
Sep 26, 2023
Merged

perf: optimize identifier parsing #295

merged 1 commit into from
Sep 26, 2023

Conversation

DaniPopes
Copy link
Member

@DaniPopes DaniPopes commented Sep 23, 2023

Motivation

We know that all identifiers in Solidity are ASCII, so we don't need to bother with multi-byte characters. As a bonus, we also get to make is_valid_identifier const (!).

Note that this was initially just going to replace s.chars() with s.as_bytes().iter().map(|x| *x as char), but we can go a step further and remove iterators to make the function const.

Godbolt

I also split the parsing benchmark into parsing and resolving, so below is the before/after.

resolve bench should be mostly noise since that is not touched.

Before:

parse/keywords          time:   [29.776 ns 29.803 ns 29.830 ns]                            
parse/complex           time:   [721.65 ns 722.08 ns 722.58 ns]                           

resolve/keywords        time:   [28.775 ns 28.798 ns 28.824 ns]                              
resolve/complex         time:   [658.11 ns 658.53 ns 658.98 ns]                             

After:

parse/keywords          time:   [27.398 ns 27.425 ns 27.453 ns]                            
                        change: [-9.6446% -9.1972% -8.8178%] (p = 0.00 < 0.05)
                        Performance has improved.
parse/complex           time:   [583.73 ns 584.82 ns 586.14 ns]                           
                        change: [-17.511% -17.085% -16.524%] (p = 0.00 < 0.05)
                        Performance has improved.

resolve/keywords        time:   [29.062 ns 29.166 ns 29.322 ns]                              
                        change: [+2.0503% +2.4201% +2.8360%] (p = 0.00 < 0.05)
                        Change within noise threshold.
resolve/complex         time:   [716.83 ns 718.96 ns 721.46 ns]                             
                        change: [+6.8756% +7.2941% +7.6725%] (p = 0.00 < 0.05)
                        Performance has regressed.

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@DaniPopes DaniPopes merged commit 7cb5bb2 into main Sep 26, 2023
18 checks passed
@DaniPopes DaniPopes deleted the dani/ident-perf branch September 26, 2023 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant