Repository navigation
[Perf][SwiftParser] Build every syntax collection without an Array - #3456
Conversation
51da0a9 to
a335b07
Compare
| while capacity < required { | ||
| capacity *= 2 | ||
| } | ||
| let grown = allocator.allocator.allocate(Element.self, count: capacity) |
There was a problem hiding this comment.
Probably BumpPointerAllocator should have reallocate(buffer:count:). If nothing was allocated after the buffer, and the current slab still has the capacity, we can just extend the buffer.
There was a problem hiding this comment.
Prototyped and measured, but it turned out that growth is rare. Over 95% of list fits in the initial capacity (4), and list creations are often nested, so the buffer is rarely the last on the slab. Not worth it.
4609b9e to
4fe9e8b
Compare
|
@swift-ci Please test |
| /// Trivially copyable, and owns nothing: whoever gathered the elements owns them | ||
| /// and must keep them alive until the collection is built. | ||
| @_spi(RawSyntax) | ||
| public struct RawSyntaxNodeList<Element: RawSyntaxNodeProtocol> { |
There was a problem hiding this comment.
Should this conform to RandomAccessCollection so callers don't need to write .buffer all the time?
There was a problem hiding this comment.
I don't think it's worth. It's meant to be built once and handed to an initializer, not used as a general collection. Most consumers are generated initializers. Adding conformance wouldn't give us much.
| var builder = RawSyntaxNodeListBuilder<Element>(initialCapacity: 1) | ||
| builder.append(element, allocator: self) | ||
| return builder.build() |
There was a problem hiding this comment.
I wonder, should we consider an inline representation for the single-element case?
There was a problem hiding this comment.
Interesting idea, I will try it in a followup
| return nil | ||
| } | ||
| let raw = RawSyntax.makeLayout(kind: .unexpectedNodes, uninitializedCount: count, arena: arena) { layout in | ||
| for (slot, node) in zip(layout.indices, nodes.lazy.compactMap({ $0 })) { |
There was a problem hiding this comment.
| for (slot, node) in zip(layout.indices, nodes.lazy.compactMap({ $0 })) { | |
| for case let (slot, node?) in zip(layout.indices, nodes) { |
There was a problem hiding this comment.
slot must be incremented for each non-nil element.
Since it was confusing, I'm changing this to
_ = layout.initialize(from: nodes.lazy.compactMap({ $0?.raw }))A collection's initializer took an `Array`, copied the elements into the arena and let the array go. A collection is almost always tiny: counting only those that hold anything, over 400 of the parser's own sources, 60% hold a single element and 94% hold three or fewer. So each was a heap allocation, a reference count and a free to carry one or two pointers, and a parse of the declaration-heavy input built 8,388 of them. CodeGeneration emits one initializer per collection, taking a `RawSyntaxNodeList`, which is a buffer and a count. The `Array` one is gone rather than joined, so nothing can quietly go back to allocating. Nothing outside SwiftParser was using it. The parser gathers into a `RawSyntaxNodeListBuilder`, in memory a `RawSyntaxNodeListAllocator` owns for the length of the parse. Deliberately not the syntax arena, whose memory lives as long as the tree: what is gathered is read once, when the collection is built, so putting it there would leave a buffer per collection alive for as long as anything holds the tree. A collection whose one element is made up while recovering from a parse error uses `list(containing:)`, and `RawUnexpectedNodesSyntax.init?(_:arena:)` lays out its node directly from the array it is given; those literal arrays, nearly all on recovery paths, are the only ones left. Both types are trivially copyable, which is why the builder takes the allocator as an argument to `append` rather than holding it: one stored class reference makes every copy of the builder retain and every destroy release, where passing it costs nothing because it comes from a stored property of the parser, which the compiler can see stays alive. The element is written through the buffer's base address rather than with `initializeElement(at:to:)`, which does not come down to a store for a generic element and measured 0.5% of a parse. A parse executes 10.5% fewer instructions on the collections input, 11.4% on the declaration-heavy one, 11.9% on the non-ASCII one and 0.5% on the corrupt one, whose time is in recovery. The tree is unchanged.
4fe9e8b to
1990610
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
A collection's initializer took an
Array, copied the elements into the arena and let the array go. A collection is almost always tiny: counting only those that hold anything, over 400 of the parser's own sources, 60% hold a single element and 94% hold three or fewer. Each was a heap allocation, a reference count and a free to carry one or two pointers, and a parse of the declaration-heavy input built 8,388 of them.Every collection's initializer now takes a
RawSyntaxNodeList, which is a pointer and a count. TheArrayaccepting initializer is removed, so nothing can quietly go back to allocating. Nothing outside SwiftParser used it.The parser gathers elements into a
RawSyntaxNodeListBuilder, in memory owned by aRawSyntaxNodeListAllocatorfor the length of the parse. This memory is deliberately not the syntax arena, which lives as long as the tree: the elements are read once, when the collection is built, so the arena would keep a dead buffer per collection for as long as anything holds the tree. Both types are trivially copyable, which is why the builder takes the allocator as an argument toappendinstead of storing it: a stored class reference would make every copy of the builder retain and every destroy release.The only arrays left are the literal ones passed to
RawUnexpectedNodesSyntax.init?(_:arena:), nearly all on recovery paths, and that initializer lays out its node from them directly.Against main (136bfa2) A parse executes 10.5-11.9% fewer instructions on valid source code, and 0.5% on a heavily corrupted one, whose time goes to recovery.
@_spi(RawSyntax)changes: everyRaw…Syntaxcollection'sinit(elements: [Element], arena:)is replaced byinit(elements: RawSyntaxNodeList<Element>, arena:), andRawSyntaxNodeListis added.