Skip to content

Conversation

rintaro
Copy link
Member

@rintaro rintaro commented Nov 7, 2022

Minimize the default memory footprint of SyntaxArena (for non-parsing). Introduce ParsingSyntaxArena as a subclass of SyntaxArena optimized for parsing.

@rintaro rintaro requested a review from ahoppen as a code owner November 7, 2022 23:58
@rintaro
Copy link
Member Author

rintaro commented Nov 7, 2022

@swift-ci Please test

@rintaro
Copy link
Member Author

rintaro commented Nov 8, 2022

cc: @jpsim Another optimization, mainly for reducing memory usage.

@rintaro
Copy link
Member Author

rintaro commented Nov 8, 2022

🤦 Forgot to rebase onto up-to-date #1053. Updated.

@rintaro
Copy link
Member Author

rintaro commented Nov 8, 2022

@swift-ci Please test

1 similar comment
@rintaro
Copy link
Member Author

rintaro commented Nov 8, 2022

@swift-ci Please test

@jpsim
Copy link
Contributor

jpsim commented Nov 8, 2022

In terms of memory usage I see a modest improvement in SwiftLint going from 159MB to 152MB. I’ll happily take it!

Comment on lines +28 to +30
public convenience init() {
self.init(slabSize: 128)
}
Copy link
Contributor

@bnbarham bnbarham Nov 8, 2022

Choose a reason for hiding this comment

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

Couldn't we just not use a BumpPtrAllocator for the non-parsing case at all? It's only ever going to have a single allocation, so it still seems wasteful to use the BumpPtrAllocator here.

I love this in general though :)

Copy link
Member Author

@rintaro rintaro Nov 8, 2022

Choose a reason for hiding this comment

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

It still may allocate multiple nodes. See SyntaxData.replacingSelf(_:arena:) and SyntaxData.replacingChild(_:at:arena:)
By this, single modifier call uses single arena, and creates multiple RawSyntax in it, from the leaf to the root. And I think that is a good thing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, fair enough. I think it'd be interesting to compare against allocating as needed but this is okay too 👍

@rintaro
Copy link
Member Author

rintaro commented Nov 8, 2022

@swift-ci Please test

@rintaro rintaro requested review from CodaFi and bnbarham November 8, 2022 03:10
Comment on lines 196 to 197
/// Parse `source` into as trivia into a list of `RawTriviaPiece` using
/// `parseTriviaFunction`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Parse `source` into as trivia into a list of `RawTriviaPiece` using
/// `parseTriviaFunction`.
/// Parse `source` into a list of `RawTriviaPiece` using `parseTriviaFunction`.

Minimize the default memory footprint of SyntaxArena. Introduce
ParsingSyntaxArena as a subclass of SyntaxArena optimized for parsing.
@rintaro
Copy link
Member Author

rintaro commented Nov 8, 2022

@swift-ci Please test

@rintaro rintaro merged commit e07287b into swiftlang:main Nov 9, 2022
@rintaro rintaro deleted the arena-for-parsing branch November 9, 2022 17:14
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