Skip to content

Commit d0cb7ad

Browse files
ahoppenahoppen
committed
[libSyntax] Eliminate loop in RawSyntax constructor
Currently, when creating a `RawSyntax` layout node, the `RawSyntax` constructor needs to iterate over all child nodes to a) sum up their sub node count b) add their arena as a child arena of the new node's arena But we are already iterating over all child nodes in every place that calls these constructors. So instead of looping twice, we can perform the above operations in the loop that already exists and pass the parameters to the `RawSyntax` constructor, which spees up `RawSyntax` node creation. To ensure the integrity of the `RawSyntax` tree, the passed in values are still validated in release builds.
1 parent cbd0206 commit d0cb7ad

File tree

8 files changed

+83
-106
lines changed

8 files changed

+83
-106
lines changed

‎include/swift/Syntax/RawSyntax.h

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -218,31 +218,41 @@ class RawSyntax final
218218
}
219219

220220
/// Constructor for creating layout nodes.
221-
/// If the node has been allocated inside the bump allocator of a
222-
/// \c SyntaxArena, that arena must be passed as \p Arena to retain the node's
223-
/// underlying storage.
221+
/// \p Children is an iterator that provides the child \c RawSyntax nodes.
222+
/// It is only traversed once.
223+
/// \p NumChildren is the number of elements provided by the \p Children
224+
/// iterator.
224225
/// If \p NodeId is \c None, the next free NodeId is used, if it is passed,
225226
/// the caller needs to assure that the node ID has not been used yet.
226-
RawSyntax(SyntaxKind Kind, ArrayRef<const RawSyntax *> Layout,
227-
size_t TextLength, SourcePresence Presence,
227+
template <typename ChildrenIteratorType>
228+
RawSyntax(SyntaxKind Kind, ChildrenIteratorType ChildrenIt,
229+
uint32_t NumChildren, SourcePresence Presence,
228230
const RC<SyntaxArena> &Arena, llvm::Optional<SyntaxNodeId> NodeId)
229-
: Arena(Arena.get()), TextLength(uint32_t(TextLength)),
231+
: Arena(Arena.get()), TextLength(0 /*computed in body*/),
230232
Presence(Presence), IsToken(false),
231-
Bits(LayoutData{uint32_t(Layout.size()),
232-
/*TotalSubNodeCount=*/0, /*set in body*/
233-
Kind}) {
233+
Bits(LayoutData{NumChildren,
234+
/*TotalSubNodeCount=*/0 /*computed in body*/, Kind}) {
234235
assert(Arena && "RawSyntax nodes must always be allocated in an arena");
235236
assert(
236237
Kind != SyntaxKind::Token &&
237238
"'token' syntax node must be constructed with dedicated constructor");
238239

239-
for (auto Child : Layout) {
240+
const RawSyntax **TrailingChildren =
241+
getTrailingObjects<const RawSyntax *>();
242+
for (uint32_t I = 0; I < NumChildren;
243+
++I, ++ChildrenIt, ++TrailingChildren) {
244+
const RawSyntax *Child = *ChildrenIt;
240245
if (Child) {
246+
// Compute TextLength and TotalSubNodeCount of this node in place.
247+
TextLength += Child->getTextLength();
241248
Bits.Layout.TotalSubNodeCount += Child->getTotalSubNodeCount() + 1;
249+
242250
// If the child is stored in a different arena, it needs to stay alive
243251
// as long as this node's arena is alive.
244252
Arena->addChildArena(Child->Arena);
245253
}
254+
255+
*TrailingChildren = Child;
246256
}
247257

248258
if (NodeId.hasValue()) {
@@ -251,10 +261,6 @@ class RawSyntax final
251261
} else {
252262
this->NodeId = NextFreeNodeId++;
253263
}
254-
255-
// Initialize layout data.
256-
std::uninitialized_copy(Layout.begin(), Layout.end(),
257-
getTrailingObjects<const RawSyntax *>());
258264
}
259265

260266
/// Constructor for creating token nodes
@@ -311,28 +317,26 @@ class RawSyntax final
311317
/// @{
312318

313319
/// Make a raw "layout" syntax node.
320+
template <typename ChildrenIteratorType>
314321
static const RawSyntax *
315-
make(SyntaxKind Kind, ArrayRef<const RawSyntax *> Layout, size_t TextLength,
322+
make(SyntaxKind Kind, ChildrenIteratorType ChildrenIt, size_t NumChildren,
316323
SourcePresence Presence, const RC<SyntaxArena> &Arena,
317324
llvm::Optional<SyntaxNodeId> NodeId = llvm::None) {
318325
assert(Arena && "RawSyntax nodes must always be allocated in an arena");
319-
auto size = totalSizeToAlloc<const RawSyntax *>(Layout.size());
326+
auto size = totalSizeToAlloc<const RawSyntax *>(NumChildren);
320327
void *data = Arena->Allocate(size, alignof(RawSyntax));
321328
return new (data)
322-
RawSyntax(Kind, Layout, TextLength, Presence, Arena, NodeId);
329+
RawSyntax(Kind, ChildrenIt, NumChildren, Presence, Arena, NodeId);
323330
}
324331

332+
/// Convenience constructor to create a raw "layout" syntax node from an
333+
/// \c llvm::ArrayRef containing the children.
325334
static const RawSyntax *
326-
makeAndCalcLength(SyntaxKind Kind, ArrayRef<const RawSyntax *> Layout,
327-
SourcePresence Presence, const RC<SyntaxArena> &Arena,
328-
llvm::Optional<SyntaxNodeId> NodeId = llvm::None) {
329-
size_t TextLength = 0;
330-
for (auto Child : Layout) {
331-
if (Child) {
332-
TextLength += Child->getTextLength();
333-
}
334-
}
335-
return make(Kind, Layout, TextLength, Presence, Arena, NodeId);
335+
make(SyntaxKind Kind, llvm::ArrayRef<const RawSyntax *> Children,
336+
SourcePresence Presence, const RC<SyntaxArena> &Arena,
337+
llvm::Optional<SyntaxNodeId> NodeId = llvm::None) {
338+
return make(Kind, Children.begin(), Children.size(), Presence, Arena,
339+
NodeId);
336340
}
337341

338342
/// Make a raw "token" syntax node.
@@ -367,7 +371,7 @@ class RawSyntax final
367371
/// Make a missing raw "layout" syntax node.
368372
static const RawSyntax *missing(SyntaxKind Kind,
369373
const RC<SyntaxArena> &Arena) {
370-
return make(Kind, {}, /*TextLength=*/0, SourcePresence::Missing, Arena);
374+
return make(Kind, {}, SourcePresence::Missing, Arena);
371375
}
372376

373377
/// Make a missing raw "token" syntax node.

‎include/swift/Syntax/Serialization/SyntaxDeserialization.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,8 +199,8 @@ template <> struct MappingTraits<const swift::RawSyntax *> {
199199
StringRef nodeIdString;
200200
in.mapRequired("id", nodeIdString);
201201
unsigned nodeId = std::atoi(nodeIdString.data());
202-
value = swift::RawSyntax::makeAndCalcLength(kind, layout, presence,
203-
input->Arena, nodeId);
202+
value =
203+
swift::RawSyntax::make(kind, layout, presence, input->Arena, nodeId);
204204
}
205205
}
206206
};

‎include/swift/Syntax/SyntaxCollection.h

Lines changed: 19 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,11 @@ class SyntaxCollection : public Syntax {
6060
private:
6161
static RC<const SyntaxData> makeData(std::initializer_list<Element> &Elements,
6262
const RC<SyntaxArena> &Arena) {
63-
std::vector<const RawSyntax *> List;
64-
List.reserve(Elements.size());
65-
for (auto &Elt : Elements)
66-
List.push_back(Elt.getRaw());
67-
auto Raw = RawSyntax::makeAndCalcLength(CollectionKind, List,
68-
SourcePresence::Present, Arena);
63+
auto RawElements = llvm::map_iterator(
64+
Elements.begin(),
65+
[](const Element &Elt) -> const RawSyntax * { return Elt.getRaw(); });
66+
auto Raw = RawSyntax::make(CollectionKind, RawElements, Elements.size(),
67+
SourcePresence::Present, Arena);
6968
return SyntaxData::makeRoot(AbsoluteRawSyntax::forRoot(Raw));
7069
}
7170

@@ -121,9 +120,8 @@ class SyntaxCollection : public Syntax {
121120
NewLayout.reserve(OldLayout.size() + 1);
122121
std::copy(OldLayout.begin(), OldLayout.end(), back_inserter(NewLayout));
123122
NewLayout.push_back(E.getRaw());
124-
auto Raw = RawSyntax::makeAndCalcLength(CollectionKind, NewLayout,
125-
getRaw()->getPresence(),
126-
getRaw()->getArena());
123+
auto Raw = RawSyntax::make(CollectionKind, NewLayout,
124+
getRaw()->getPresence(), getRaw()->getArena());
127125
return SyntaxCollection<CollectionKind, Element>(Data->replacingSelf(Raw));
128126
}
129127

@@ -133,9 +131,8 @@ class SyntaxCollection : public Syntax {
133131
SyntaxCollection<CollectionKind, Element> removingLast() const {
134132
assert(!empty());
135133
auto NewLayout = getRaw()->getLayout().drop_back();
136-
auto Raw = RawSyntax::makeAndCalcLength(CollectionKind, NewLayout,
137-
getRaw()->getPresence(),
138-
getRaw()->getArena());
134+
auto Raw = RawSyntax::make(CollectionKind, NewLayout,
135+
getRaw()->getPresence(), getRaw()->getArena());
139136
return SyntaxCollection<CollectionKind, Element>(Data->replacingSelf(Raw));
140137
}
141138

@@ -146,9 +143,8 @@ class SyntaxCollection : public Syntax {
146143
std::vector<const RawSyntax *> NewLayout = {E.getRaw()};
147144
std::copy(OldLayout.begin(), OldLayout.end(),
148145
std::back_inserter(NewLayout));
149-
auto Raw = RawSyntax::makeAndCalcLength(CollectionKind, NewLayout,
150-
getRaw()->getPresence(),
151-
getRaw()->getArena());
146+
auto Raw = RawSyntax::make(CollectionKind, NewLayout,
147+
getRaw()->getPresence(), getRaw()->getArena());
152148
return SyntaxCollection<CollectionKind, Element>(Data->replacingSelf(Raw));
153149
}
154150

@@ -158,9 +154,8 @@ class SyntaxCollection : public Syntax {
158154
SyntaxCollection<CollectionKind, Element> removingFirst() const {
159155
assert(!empty());
160156
auto NewLayout = getRaw()->getLayout().drop_front();
161-
auto Raw = RawSyntax::makeAndCalcLength(CollectionKind, NewLayout,
162-
getRaw()->getPresence(),
163-
getRaw()->getArena());
157+
auto Raw = RawSyntax::make(CollectionKind, NewLayout,
158+
getRaw()->getPresence(), getRaw()->getArena());
164159
return SyntaxCollection<CollectionKind, Element>(Data->replacingSelf(Raw));
165160
}
166161

@@ -179,9 +174,8 @@ class SyntaxCollection : public Syntax {
179174
NewLayout.push_back(E.getRaw());
180175
std::copy(OldLayout.begin() + i, OldLayout.end(),
181176
std::back_inserter(NewLayout));
182-
auto Raw = RawSyntax::makeAndCalcLength(CollectionKind, NewLayout,
183-
getRaw()->getPresence(),
184-
getRaw()->getArena());
177+
auto Raw = RawSyntax::make(CollectionKind, NewLayout,
178+
getRaw()->getPresence(), getRaw()->getArena());
185179
return SyntaxCollection<CollectionKind, Element>(Data->replacingSelf(Raw));
186180
}
187181

@@ -192,16 +186,15 @@ class SyntaxCollection : public Syntax {
192186
auto iterator = NewLayout.begin();
193187
std::advance(iterator, i);
194188
NewLayout.erase(iterator);
195-
auto Raw = RawSyntax::makeAndCalcLength(CollectionKind, NewLayout,
196-
getRaw()->getPresence(),
197-
getRaw()->getArena());
189+
auto Raw = RawSyntax::make(CollectionKind, NewLayout,
190+
getRaw()->getPresence(), getRaw()->getArena());
198191
return SyntaxCollection<CollectionKind, Element>(Data->replacingSelf(Raw));
199192
}
200193

201194
/// Return an empty syntax collection of this type.
202195
SyntaxCollection<CollectionKind, Element> cleared() const {
203-
auto Raw = RawSyntax::makeAndCalcLength(
204-
CollectionKind, {}, getRaw()->getPresence(), getRaw()->getArena());
196+
auto Raw = RawSyntax::make(CollectionKind, {}, getRaw()->getPresence(),
197+
getRaw()->getArena());
205198
return SyntaxCollection<CollectionKind, Element>(Data->replacingSelf(Raw));
206199
}
207200

‎lib/Syntax/RawSyntax.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,7 @@ const RawSyntax *RawSyntax::append(const RawSyntax *NewLayoutElement) const {
104104
NewLayout.reserve(Layout.size() + 1);
105105
std::copy(Layout.begin(), Layout.end(), std::back_inserter(NewLayout));
106106
NewLayout.push_back(NewLayoutElement);
107-
return RawSyntax::makeAndCalcLength(getKind(), NewLayout,
108-
SourcePresence::Present, Arena);
107+
return RawSyntax::make(getKind(), NewLayout, SourcePresence::Present, Arena);
109108
}
110109

111110
const RawSyntax *
@@ -123,8 +122,7 @@ RawSyntax::replacingChild(CursorIndex Index,
123122
std::copy(Layout.begin() + Index + 1, Layout.end(),
124123
std::back_inserter(NewLayout));
125124

126-
return RawSyntax::makeAndCalcLength(getKind(), NewLayout, getPresence(),
127-
Arena);
125+
return RawSyntax::make(getKind(), NewLayout, getPresence(), Arena);
128126
}
129127

130128
void RawSyntax::print(llvm::raw_ostream &OS, SyntaxPrintOptions Opts) const {

‎lib/Syntax/SyntaxBuilders.cpp.gyb

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,9 @@ ${node.name}Builder &
4545
${node.name}Builder::add${child_elt}(${child_elt_type} ${child_elt}) {
4646
auto &raw = Layout[cursorIndex(${node.name}::Cursor::${child.name})];
4747
if (!raw)
48-
raw = RawSyntax::makeAndCalcLength(SyntaxKind::${child_node.syntax_kind},
49-
{${child_elt}.getRaw()},
50-
SourcePresence::Present, Arena);
48+
raw = RawSyntax::make(SyntaxKind::${child_node.syntax_kind},
49+
{${child_elt}.getRaw()}, SourcePresence::Present,
50+
Arena);
5151
else
5252
raw = raw->append(${child_elt}.getRaw());
5353
return *this;
@@ -64,8 +64,8 @@ ${node.name}Builder::build() {
6464
% end
6565
% end
6666
% end
67-
auto raw = RawSyntax::makeAndCalcLength(SyntaxKind::${node.syntax_kind},
68-
Layout, SourcePresence::Present, Arena);
67+
auto raw = RawSyntax::make(SyntaxKind::${node.syntax_kind}, Layout,
68+
SourcePresence::Present, Arena);
6969
return makeRoot<${node.name}>(raw);
7070
}
7171

‎lib/Syntax/SyntaxFactory.cpp.gyb

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,12 @@ TokenSyntax SyntaxFactory::makeToken(tok Kind, StringRef Text,
3737

3838
UnknownSyntax
3939
SyntaxFactory::makeUnknownSyntax(llvm::ArrayRef<TokenSyntax> Tokens) {
40-
std::vector<const RawSyntax *> Layout;
41-
Layout.reserve(Tokens.size());
42-
for (auto &Token : Tokens) {
43-
Layout.push_back(Token.getRaw());
44-
}
45-
auto Raw = RawSyntax::makeAndCalcLength(SyntaxKind::Unknown, Layout,
46-
SourcePresence::Present, Arena);
40+
auto RawTokens = llvm::map_iterator(Tokens.begin(),
41+
[](const TokenSyntax &Token) -> const RawSyntax * {
42+
return Token.getRaw();
43+
});
44+
auto Raw = RawSyntax::make(SyntaxKind::Unknown, RawTokens, Tokens.size(),
45+
SourcePresence::Present, Arena);
4746
return makeRoot<UnknownSyntax>(Raw);
4847
}
4948

@@ -135,15 +134,13 @@ const RawSyntax *SyntaxFactory::createRaw(
135134
% end
136135
if (I != Elements.size())
137136
return nullptr;
138-
return RawSyntax::makeAndCalcLength(Kind, Layout, SourcePresence::Present,
139-
Arena);
137+
return RawSyntax::make(Kind, Layout, SourcePresence::Present, Arena);
140138
% elif node.is_syntax_collection():
141139
for (auto &E : Elements) {
142140
if (!canServeAsCollectionMemberRaw(SyntaxKind::${node.syntax_kind}, E))
143141
return nullptr;
144142
}
145-
return RawSyntax::makeAndCalcLength(Kind, Elements, SourcePresence::Present,
146-
Arena);
143+
return RawSyntax::make(Kind, Elements, SourcePresence::Present, Arena);
147144
% else:
148145
return nullptr;
149146
% end
@@ -178,7 +175,7 @@ Optional<Syntax> SyntaxFactory::createSyntax(SyntaxKind Kind,
178175
% child_params = ', '.join(child_params)
179176
${node.name}
180177
SyntaxFactory::make${node.syntax_kind}(${child_params}) {
181-
auto Raw = RawSyntax::makeAndCalcLength(SyntaxKind::${node.syntax_kind}, {
178+
auto Raw = RawSyntax::make(SyntaxKind::${node.syntax_kind}, {
182179
% for child in node.children:
183180
% if child.is_optional:
184181
${child.name}.hasValue() ? ${child.name}->getRaw() : nullptr,
@@ -198,8 +195,8 @@ SyntaxFactory::make${node.syntax_kind}(
198195
for (auto &element : elements) {
199196
layout.push_back(element.getRaw());
200197
}
201-
auto raw = RawSyntax::makeAndCalcLength(SyntaxKind::${node.syntax_kind},
202-
layout, SourcePresence::Present, Arena);
198+
auto raw = RawSyntax::make(SyntaxKind::${node.syntax_kind}, layout,
199+
SourcePresence::Present, Arena);
203200
return makeRoot<${node.name}>(raw);
204201
}
205202
% end
@@ -214,7 +211,7 @@ SyntaxFactory::makeBlank${node.syntax_kind}() {
214211
${make_missing_child(child)},
215212
% end
216213
% end
217-
}, /*TextLength=*/0, SourcePresence::Present, Arena);
214+
}, SourcePresence::Present, Arena);
218215
return makeRoot<${node.name}>(raw);
219216
}
220217
% end

‎lib/Syntax/SyntaxNodes.cpp.gyb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ ${node.name} ${node.name}::add${child_elt}(${child_elt_type} ${child_elt}) {
8686
if (raw)
8787
raw = raw->append(${child_elt}.getRaw());
8888
else
89-
raw = RawSyntax::makeAndCalcLength(SyntaxKind::${child_node.syntax_kind},
89+
raw = RawSyntax::make(SyntaxKind::${child_node.syntax_kind},
9090
{${child_elt}.getRaw()}, SourcePresence::Present, raw->getArena());
9191
return ${node.name}(Data->replacingChild(raw, Cursor::${child.name}));
9292
}

‎lib/SyntaxParse/SyntaxTreeCreator.cpp

Lines changed: 10 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -141,18 +141,10 @@ SyntaxTreeCreator::recordMissingToken(tok kind, SourceLoc loc) {
141141
OpaqueSyntaxNode
142142
SyntaxTreeCreator::recordRawSyntax(syntax::SyntaxKind kind,
143143
ArrayRef<OpaqueSyntaxNode> elements) {
144-
SmallVector<const RawSyntax *, 16> parts;
145-
parts.reserve(elements.size());
146-
size_t TextLength = 0;
147-
for (OpaqueSyntaxNode opaqueN : elements) {
148-
auto Raw = static_cast<const RawSyntax *>(opaqueN);
149-
parts.push_back(Raw);
150-
if (Raw) {
151-
TextLength += Raw->getTextLength();
152-
}
153-
}
154-
auto raw =
155-
RawSyntax::make(kind, parts, TextLength, SourcePresence::Present, Arena);
144+
const RawSyntax *const *rawChildren =
145+
reinterpret_cast<const RawSyntax *const *>(elements.begin());
146+
auto raw = RawSyntax::make(kind, rawChildren, elements.size(),
147+
SourcePresence::Present, Arena);
156148
return static_cast<OpaqueSyntaxNode>(raw);
157149
}
158150

@@ -194,19 +186,12 @@ OpaqueSyntaxNode SyntaxTreeCreator::makeDeferredLayout(
194186
const MutableArrayRef<ParsedRawSyntaxNode> &parsedChildren) {
195187
assert(!IsMissing && "Missing layout nodes not implemented yet");
196188

197-
SmallVector<const RawSyntax *, 16> children;
198-
children.reserve(parsedChildren.size());
199-
200-
size_t TextLength = 0;
201-
for (size_t i = 0; i < parsedChildren.size(); ++i) {
202-
auto Raw = static_cast<const RawSyntax *>(parsedChildren[i].takeData());
203-
if (Raw) {
204-
TextLength += Raw->getTextLength();
205-
}
206-
children.push_back(Raw);
207-
}
208-
209-
auto raw = RawSyntax::make(kind, children, TextLength,
189+
auto rawChildren = llvm::map_iterator(
190+
parsedChildren.begin(),
191+
[](ParsedRawSyntaxNode &parsedChild) -> const RawSyntax * {
192+
return static_cast<const RawSyntax *>(parsedChild.takeData());
193+
});
194+
auto raw = RawSyntax::make(kind, rawChildren, parsedChildren.size(),
210195
SourcePresence::Present, Arena);
211196
return static_cast<OpaqueSyntaxNode>(raw);
212197
}

0 commit comments

Comments
 (0)