Skip to content

[libSyntax] Store the token's text in the SyntaxArena #35733

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 2 commits into from
Feb 11, 2021

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Feb 3, 2021

This PR contains commits from #35649


Do the same thing that we are already doing for trivia: Since RawSyntax nodes always live inside a SyntaxArena, we don't need to tail-allocate an OwnedString to store the token's text. Instead we can just copy it to the SyntaxArena. If we copy the entire source buffer to the syntax arena at the start of parsing, this means that no more copies are required later on. Plus we also avoid ref-counting the OwnedString which should also increase performance.

Performance checklist (performed on my local machine)

  • No regression during compilation
  • No regression during code completion
  • No regression during SwiftSyntax parsing

@ahoppen ahoppen force-pushed the raw-syntax-text-storage branch from 8be3707 to 8355d88 Compare February 3, 2021 14:16
@ahoppen ahoppen marked this pull request as ready for review February 3, 2021 16:37
@ahoppen ahoppen force-pushed the raw-syntax-text-storage branch 3 times, most recently from 748ca47 to 2509f59 Compare February 4, 2021 13:30
@swiftlang swiftlang deleted a comment from swift-ci Feb 4, 2021
@ahoppen
Copy link
Member Author

ahoppen commented Feb 4, 2021

@swift-ci Please test

rintaro
rintaro previously approved these changes Feb 5, 2021
Copy link
Member

@rintaro rintaro left a comment

Choose a reason for hiding this comment

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

LGTM!

@rintaro
Copy link
Member

rintaro commented Feb 5, 2021

Ah sorry wrong PR

@rintaro rintaro dismissed their stale review February 5, 2021 18:26

Wrong PR

Do the same thing that we are already doing for trivia: Since RawSyntax
nodes always live inside a SyntaxArena, we don't need to tail-allocate
an OwnedString to store the token's text. Instead we can just copy it
to the SyntaxArena. If we copy the entire source buffer to the syntax
arena at the start of parsing, this means that no more copies are
required later on. Plus we also avoid ref-counting the OwnedString which
should also increase performance.
This decreases the size of RawSyntax nodes from 88 to 64 bytes by
- Avoiding some padding by moving RefCount further up
- Limiting the length of tokens and their trivia to 32 bits. We would
  hit this limit with files >4GB but we also hit this limit at other
  places like the TextLength property in the Common bits.
@ahoppen ahoppen force-pushed the raw-syntax-text-storage branch from 2509f59 to c1d65de Compare February 10, 2021 08:50
@ahoppen
Copy link
Member Author

ahoppen commented Feb 10, 2021

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - c1d65de

@ahoppen
Copy link
Member Author

ahoppen commented Feb 10, 2021

@swift-ci Please smoke test macOS platform

Copy link
Member

@rintaro rintaro left a comment

Choose a reason for hiding this comment

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

Thanks!

@ahoppen ahoppen merged commit 512411b into swiftlang:main Feb 11, 2021
@ahoppen ahoppen deleted the raw-syntax-text-storage branch February 11, 2021 14:33
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