Skip to content

Commit 58ef1eb

Browse files
authored
[libc] Bound the worst-case stack usage in qsort(). (#110849)
Previously, the Quicksort implementation was written in the obvious way: after each partitioning step, it explicitly recursed twice to sort the two sublists. Now it compares the two sublists' sizes, and recurses only to sort the smaller one. To handle the larger list it loops back round to the top of the function, so as to handle it within the existing stack frame. This means that every recursive call is handling a list at most half that of its caller. So the maximum recursive call depth is O(lg N). Otherwise, in Quicksort's bad cases where each partition step peels off a small constant number of array elements, the stack usage could grow linearly with the array being sorted, i.e. it might be Θ(N). I tested this code by manually constructing a List Of Doom that causes this particular quicksort implementation to hit its worst case, and confirming that it recursed very deeply in the old code and doesn't in the new code. But I haven't added that list to the test suite, because the List Of Doom has to be constructed in a way based on every detail of the quicksort algorithm (pivot choice and partitioning strategy), so it would silently stop being a useful regression test as soon as any detail changed.
1 parent 975da02 commit 58ef1eb

File tree

2 files changed

+33
-12
lines changed

2 files changed

+33
-12
lines changed

libc/src/stdlib/qsort_data.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,15 @@ class Array {
8989
size_t size() const { return array_size; }
9090

9191
// Make an Array starting at index |i| and size |s|.
92-
Array make_array(size_t i, size_t s) const {
92+
LIBC_INLINE Array make_array(size_t i, size_t s) const {
9393
return Array(get(i), s, elem_size, compare);
9494
}
95+
96+
// Reset this Array to point at a different interval of the same items.
97+
LIBC_INLINE void reset_bounds(uint8_t *a, size_t s) {
98+
array = a;
99+
array_size = s;
100+
}
95101
};
96102

97103
using SortingRoutine = void(const Array &);

libc/src/stdlib/quick_sort.h

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ namespace LIBC_NAMESPACE_DECL {
1919
namespace internal {
2020

2121
// A simple quicksort implementation using the Hoare partition scheme.
22-
static size_t partition(const Array &array) {
22+
LIBC_INLINE size_t partition(const Array &array) {
2323
const size_t array_size = array.size();
2424
size_t pivot_index = array_size / 2;
2525
uint8_t *pivot = array.get(pivot_index);
@@ -59,17 +59,32 @@ static size_t partition(const Array &array) {
5959
}
6060
}
6161

62-
LIBC_INLINE void quick_sort(const Array &array) {
63-
const size_t array_size = array.size();
64-
if (array_size <= 1)
65-
return;
66-
size_t split_index = partition(array);
67-
if (array_size <= 2) {
68-
// The partition operation sorts the two element array.
69-
return;
62+
LIBC_INLINE void quick_sort(Array array) {
63+
while (true) {
64+
const size_t array_size = array.size();
65+
if (array_size <= 1)
66+
return;
67+
size_t split_index = partition(array);
68+
if (array_size == 2)
69+
// The partition operation sorts the two element array.
70+
return;
71+
72+
// Make Arrays describing the two sublists that still need sorting.
73+
Array left = array.make_array(0, split_index);
74+
Array right = array.make_array(split_index, array.size() - split_index);
75+
76+
// Recurse to sort the smaller of the two, and then loop round within this
77+
// function to sort the larger. This way, recursive call depth is bounded
78+
// by log2 of the total array size, because every recursive call is sorting
79+
// a list at most half the length of the one in its caller.
80+
if (left.size() < right.size()) {
81+
quick_sort(left);
82+
array.reset_bounds(right.get(0), right.size());
83+
} else {
84+
quick_sort(right);
85+
array.reset_bounds(left.get(0), left.size());
86+
}
7087
}
71-
quick_sort(array.make_array(0, split_index));
72-
quick_sort(array.make_array(split_index, array.size() - split_index));
7388
}
7489

7590
} // namespace internal

0 commit comments

Comments
 (0)