Skip to content

Commit c5c78c5

Browse files
committed
Add an insertAtFront method to the list merger.
The existing insert method preserves insertion order, but the atomic queue use case actually wants to *reverse* the natural insertion order.
1 parent 21350a9 commit c5c78c5

File tree

2 files changed

+203
-33
lines changed

2 files changed

+203
-33
lines changed

include/swift/Basic/ListMerger.h

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,99 @@ class ListMerger {
133133
setLastInsertionPoint(newNode, /*known last of equals*/ true);
134134
}
135135

136+
/// Add a single node to this merger's current list.
137+
///
138+
/// The next reference of the node will be overwritten and does not
139+
/// need to be meaningful.
140+
///
141+
/// The relative order of nodes in the current list will not change,
142+
/// and if there are nodes in the current list which compare equal
143+
/// to the new node, it will be inserted *before* them.
144+
///
145+
/// This is useful for the pattern where nodes are naturally encountered
146+
/// in the opposite of their desired order in the final list and
147+
/// need to be reversed. It generally doesn't make any sense to mix
148+
/// this with calls to insert or merge on the same merger.
149+
void insertAtFront(Node newNode) {
150+
assert(newNode && "inserting a null node");
151+
152+
auto insertBetween = [newNode, this](Node prev, Node next) {
153+
if (prev) {
154+
assert(NodeTraits::getNext(prev) == next);
155+
assert(NodeTraits::compare(prev, newNode) < 0);
156+
NodeTraits::setNext(prev, newNode);
157+
} else {
158+
assert(root == next);
159+
root = newNode;
160+
}
161+
162+
assert(!next || NodeTraits::compare(newNode, next) <= 0);
163+
NodeTraits::setNext(newNode, next);
164+
setLastInsertionPoint(prev, /*known last of equals*/ true);
165+
};
166+
167+
Node prev = Node();
168+
Node cur = root;
169+
170+
// If we have a previous insertion point, check for the presumed-common
171+
// case that we're inserting something that should immediately follow it.
172+
if (auto lastIP = lastInsertionPoint) {
173+
lastIP = findLastOfEqualsFromLastIP(lastIP);
174+
175+
// Compare against the next node after lastIP, if it exists.
176+
if (Node nextAfterLastIP = NodeTraits::getNext(lastIP)) {
177+
int comparison = NodeTraits::compare(nextAfterLastIP, newNode);
178+
179+
// If the new node compares equal to the next node, insert here.
180+
if (comparison == 0) {
181+
insertBetween(lastIP, nextAfterLastIP);
182+
return;
183+
}
184+
185+
// If the new node should follow the next node, start scanning
186+
// after it.
187+
if (comparison < 0) {
188+
prev = nextAfterLastIP;
189+
cur = NodeTraits::getNext(nextAfterLastIP);
190+
}
191+
192+
// Otherwise, we'll need to scan from the beginning.
193+
194+
// If there is no next node, compare against the previous.
195+
} else {
196+
int comparison = NodeTraits::compare(lastIP, newNode);
197+
198+
// If the new node should follow the last node, we can
199+
// insert here.
200+
if (comparison < 0) {
201+
insertBetween(lastIP, Node());
202+
return;
203+
}
204+
205+
// Otherwise, we'll need to scan from the beginning.
206+
}
207+
}
208+
209+
assert(!prev || NodeTraits::compare(prev, newNode) < 0);
210+
211+
// Scan forward, looking for a node which the new node must be
212+
// inserted prior to.
213+
// Invariant: prev < newNode, if prev exists
214+
while (cur) {
215+
// Compare the new node against the current IP.
216+
int comparison = NodeTraits::compare(cur, newNode);
217+
218+
// If the new node isn't strictly greater than cur, insert here.
219+
if (comparison >= 0) break;
220+
221+
// Otherwise, continue.
222+
prev = cur;
223+
cur = NodeTraits::getNext(prev);
224+
}
225+
226+
insertBetween(prev, cur);
227+
}
228+
136229
/// Add a sorted list of nodes to this merger's current list.
137230
/// The list must be well-formed (i.e. appropriately terminated).
138231
///

utils/test-list-merger/TestListMerger.cpp

Lines changed: 110 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,53 @@ static int compare_unsigned(unsigned lhs, unsigned rhs) {
1212
}
1313

1414
namespace {
15+
enum EntryOrder {
16+
creationOrder,
17+
reverseCreationOrder
18+
};
19+
1520
struct Entry {
1621
unsigned id;
1722
unsigned value;
1823
Entry *next;
1924
};
2025

26+
class EntryFactory {
27+
std::vector<std::unique_ptr<Entry>> entries;
28+
unsigned nextID = 0;
29+
public:
30+
Entry *create(unsigned value) {
31+
auto entry = new Entry{nextID++, value, nullptr};
32+
entries.emplace_back(entry);
33+
return entry;
34+
}
35+
36+
/// Sort the entries in this list.
37+
///
38+
/// \param reverseCreationOrder - if true, then order equal-value
39+
/// nodes in the reverse of creation order; otherwise
40+
/// creator order of creation order
41+
void sort(EntryOrder order) {
42+
std::sort(entries.begin(), entries.end(),
43+
[=](const std::unique_ptr<Entry> &lhs,
44+
const std::unique_ptr<Entry> &rhs) {
45+
if (lhs->value != rhs->value) return lhs->value < rhs->value;
46+
return order == creationOrder
47+
? lhs->id < rhs->id
48+
: lhs->id > rhs->id;
49+
});
50+
}
51+
52+
void checkSameAs(Entry *list) {
53+
for (auto &entry: entries) {
54+
std::cout << " " << list->value << " (" << list->id << ")\n";
55+
assert(list == entry.get());
56+
list = list->next;
57+
};
58+
assert(list == nullptr);
59+
}
60+
};
61+
2162
struct EntryListTraits {
2263
static Entry *getNext(Entry *e) { return e->next; }
2364
static void setNext(Entry *e, Entry *next) { e->next = next; }
@@ -26,6 +67,8 @@ struct EntryListTraits {
2667
}
2768
};
2869

70+
using EntryListMerger = ListMerger<Entry*, EntryListTraits>;
71+
2972
enum Op {
3073
insert,
3174
beginMerge,
@@ -54,12 +97,12 @@ struct Instruction {
5497

5598
template <class T>
5699
static std::ostream &operator<<(std::ostream &str, llvm::ArrayRef<T> list) {
57-
str << "[";
100+
str << "{";
58101
for (auto b = list.begin(), i = b, e = list.end(); i != e; ++i) {
59102
if (i != b) str << ", ";
60103
str << *i;
61104
}
62-
str << "]";
105+
str << "}";
63106
return str;
64107
}
65108

@@ -68,12 +111,9 @@ static std::ostream &operator<<(std::ostream &str, const std::vector<T> &list) {
68111
return (str << llvm::makeArrayRef(list));
69112
}
70113

71-
static void runTest(llvm::ArrayRef<Instruction> values) {
72-
std::vector<std::unique_ptr<Entry>> entries;
73-
74-
ListMerger<Entry*, EntryListTraits> merger;
75-
76-
unsigned nextID = 0;
114+
static void runInsertAndMergeTest(llvm::ArrayRef<Instruction> values) {
115+
EntryFactory entries;
116+
EntryListMerger merger;
77117

78118
// Between beginMerge and endMerge instructions, values don't get
79119
// inserted immediately: they build up into a separate list of items
@@ -86,8 +126,7 @@ static void runTest(llvm::ArrayRef<Instruction> values) {
86126
switch (inst.op) {
87127
case insert: {
88128
// Create the new entry.
89-
Entry *entry = new Entry{nextID++, inst.value, nullptr};
90-
entries.emplace_back(entry);
129+
Entry *entry = entries.create(inst.value);
91130

92131
// If we're building a merge list, append to the end of it.
93132
if (lastMergeEntry) {
@@ -121,38 +160,48 @@ static void runTest(llvm::ArrayRef<Instruction> values) {
121160
}
122161
assert(!lastMergeEntry && "ended while still building a merge list");
123162

124-
// Do a stable sort of the entries.
125-
std::stable_sort(entries.begin(), entries.end(),
126-
[](const std::unique_ptr<Entry> &lhs,
127-
const std::unique_ptr<Entry> &rhs) {
128-
return (lhs->value < rhs->value);
129-
});
130-
131-
// Make sure that we end up with the same list.
132-
auto list = merger.release();
133-
for (auto &entry : entries) {
134-
std::cout << " " << list->value << " (" << list->id << ")\n";
135-
assert(list == entry.get());
136-
list = list->next;
163+
entries.sort(creationOrder);
164+
entries.checkSameAs(merger.release());
165+
}
166+
167+
static void runInsertAtFrontTest(llvm::ArrayRef<unsigned> values) {
168+
EntryFactory entries;
169+
EntryListMerger merger;
170+
for (auto value: values) {
171+
merger.insertAtFront(entries.create(value));
137172
}
138-
assert(list == nullptr);
173+
entries.sort(reverseCreationOrder);
174+
entries.checkSameAs(merger.release());
139175
}
140176

141-
int main() {
142-
runTest({ 5, 0, 3, 0, 1, 0, 7 });
177+
static void runConcreteTests() {
178+
runInsertAndMergeTest({ 5, 0, 3, 0, 1, 0, 7 });
179+
}
143180

181+
namespace {
182+
struct TestConfig {
183+
unsigned numTests;
184+
unsigned numEntries;
185+
unsigned maxValue;
186+
};
187+
188+
}
189+
190+
static void runInsertAndMergeTests(const TestConfig &config) {
144191
std::random_device randomDevice;
145192
std::default_random_engine e(randomDevice());
146-
std::uniform_int_distribution<unsigned> dist(0, 20);
193+
std::uniform_int_distribution<unsigned> valueDist(0, config.maxValue);
194+
// Chance of entering or exiting a merge.
195+
std::uniform_int_distribution<unsigned> mergeDist(0, 20);
147196

148197
std::vector<Instruction> ins;
149-
for (unsigned testN = 0; testN < 1000; ++testN) {
198+
for (unsigned testN = 0; testN < config.numTests; ++testN) {
150199
ins.clear();
151200

152201
const size_t noMerge = -1;
153202
size_t mergeStart = noMerge;
154-
for (unsigned i = 0; i < 2000 || mergeStart != noMerge; ++i) {
155-
if (dist(e) == 0) {
203+
for (unsigned i = 0; i < config.numEntries || mergeStart != noMerge; ++i) {
204+
if (mergeDist(e) == 0) {
156205
if (mergeStart != noMerge) {
157206
std::sort(ins.begin() + mergeStart, ins.end(),
158207
[](const Instruction &lhs, const Instruction &rhs) {
@@ -165,11 +214,39 @@ int main() {
165214
mergeStart = ins.size();
166215
}
167216
} else {
168-
ins.push_back(dist(e));
217+
ins.push_back(valueDist(e));
169218
}
170219
}
171220

172-
std::cout << ins << std::endl;
173-
runTest(ins);
221+
std::cout << "runInsertAndMergeTest(" << ins << ");" << std::endl;
222+
runInsertAndMergeTest(ins);
223+
}
224+
}
225+
226+
static void runInsertAtFrontTests(const TestConfig &config) {
227+
std::random_device randomDevice;
228+
std::default_random_engine e(randomDevice());
229+
std::uniform_int_distribution<unsigned> valueDist(0, config.maxValue);
230+
231+
std::vector<unsigned> ins;
232+
for (unsigned testN = 0; testN < config.numTests; ++testN) {
233+
ins.clear();
234+
for (unsigned i = 0; i < config.numEntries; ++i) {
235+
ins.push_back(valueDist(e));
236+
}
237+
238+
std::cout << "runInsertAtFrontTest(" << ins << ");" << std::endl;
239+
runInsertAtFrontTest(ins);
174240
}
175241
}
242+
243+
int main() {
244+
TestConfig config = {
245+
.numTests = 1000,
246+
.numEntries = 2000,
247+
.maxValue = 3
248+
};
249+
runConcreteTests();
250+
runInsertAndMergeTests(config);
251+
runInsertAtFrontTests(config);
252+
}

0 commit comments

Comments
 (0)