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.

Sign up for GitHub

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

Jump to bottom

[Perf][WhitespaceLinter] Use hand crafted "is whitespace" function #915

Merged
merged 1 commit into from
Jan 13, 2025
Merged

[Perf][WhitespaceLinter] Use hand crafted "is whitespace" function #915

merged 1 commit into from
Jan 13, 2025

Conversation

rintaro
Copy link
Member

@rintaro rintaro commented Jan 13, 2025 •
edited
Loading

  • UnicodeScalar(_:) on arbitrary UTF8 code point was wrong. It only works correctly if the code point is < 0x80
  • UnicodeScalar.Properties.isWhitespace is slow. Profiling lint showed it was taking 13.6% of the entire run time
  • Whitespaces in Unicode "Basic Latin" block are well defined, there's no need to consult UnicodeScalar.Properties
Screenshot 2025-01-13 at 10 25 12 AM 
swift-format lint --recursive /tmp/swift-syntax-509.0.0`
// Before
Instructions executed: 54264413578
// After
Instructions executed: 49121489746

@rintaro
[WhitespaceLinter] Use hand crafted "is whitespace" function
83b5f01 
* `UnicodeScalar(_:)` on arbitrary UTF8 code point was wrong. It only
  works correctly if the code point is < 0x80
* `UnicodeScalar.Properties.isWhitespace` is slow. Profiling 'lint'
  shows it's taking 13.6 of the entire time
* Whitespaces in Unicode "Basic Latin" block are well defined, there's
  no need to consult `UnicodeScalar.Properties`
@rintaro
Copy link
Member Author

rintaro commented Jan 13, 2025

@swift-ci Please test

allevato
allevato approved these changes Jan 13, 2025
View reviewed changes
Copy link
Member

@allevato allevato left a comment

Choose a reason for hiding this comment

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

Very nice improvement, thanks!

(I'm surprised that looking up a Boolean property in the Unicode tables that are baked into the runtime is that expensive, though...)

@bnbarham
Copy link
Contributor

Nice, thanks @rintaro!

@bnbarham bnbarham merged commit c0b518e into swiftlang:main Jan 13, 2025
19 checks passed
@rintaro rintaro deleted the lint-iswhitespace branch January 13, 2025 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnbarham bnbarham bnbarham approved these changes

@allevato allevato allevato approved these changes

@ahoppen ahoppen Awaiting requested review from ahoppen ahoppen is a code owner

@shawnhyam shawnhyam Awaiting requested review from shawnhyam shawnhyam is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rintaro @bnbarham @allevato