Skip to content

[SourceLocationConverter] Performance improvement #2663

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

Merged
merged 4 commits into from
May 23, 2024

Conversation

rintaro
Copy link
Member

@rintaro rintaro commented May 22, 2024

  • Stop using TokenSequence. Instead, visit RawSyntax tree manually
  • Avoid parsing trivia, and creating Array of RawTriviaPiece. Instead, scan the whole token text directly if available
  • Collect #sourceLocation() directives in one-pass

@rintaro rintaro requested review from ahoppen and bnbarham as code owners May 22, 2024 20:32
@rintaro
Copy link
Member Author

rintaro commented May 22, 2024

@swift-ci Please test

@rintaro rintaro requested a review from hamishknight May 22, 2024 21:13
var position: AbsolutePosition = .startOfFile
let addLine = { (lineLength: SourceLength) in
var sourceLocationDirectives: [(sourceLine: Int, arguments: SourceLocationDirectiveArguments?)] = []
let prefix = tree.raw.forEachLineLength { lineLength in
Copy link
Member

Choose a reason for hiding this comment

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

prefix doesn’t really make sense as a variable name here, right? Shouldn’t it be sourceFileLength or something? Also, the usage of prefix in RawSyntax.forEachLineLength + the comment on it seem pretty confusing to me. Not your doing but might be worth to clean that up along the way.

Copy link
Member Author

Choose a reason for hiding this comment

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

This prefix is more like lastLineLength. I changed the naming so.
I didn't changed the naming in forEachLineLength functions, because I wasn't able to come up with better naming. It's like "the length of the current line so far"

@rintaro rintaro force-pushed the perf-computelines branch from e54f8d1 to 6a81e49 Compare May 23, 2024 02:06
@rintaro
Copy link
Member Author

rintaro commented May 23, 2024

@swift-ci Please test

@rintaro
Copy link
Member Author

rintaro commented May 23, 2024

@swift-ci Please test Windows

@rintaro
Copy link
Member Author

rintaro commented May 23, 2024

@swift-ci Please test

@rintaro
Copy link
Member Author

rintaro commented May 23, 2024

@swift-ci Please test macOS

@rintaro rintaro merged commit 83ee689 into swiftlang:main May 23, 2024
3 checks passed
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.

3 participants