Skip to content

solve for accidental quadratic performance from min max methods #267

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vanvoorden wants to merge 1 commit into
base: main
Choose a base branch
from
+5 −5
Open

solve for accidental quadratic performance from min max methods #267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Guides/MinMax.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ extension Sequence where Element: Comparable {
The algorithm used for minimal- or maximal-ordered subsets is based on
[Soroush Khanlou's research on this matter](https://khanlou.com/2018/12/analyzing-complexity/).
The total complexity is `O(k log k + nk)`, which will result in a runtime close
to `O(n)` if *k* is a small amount. If *k* is a large amount (more than 10% of
to `O(n)` if *k* is a small amount. If *k* is a large amount (more than log n of
Copy link
Copy Markdown
Contributor

@xwu xwu Dec 30, 2025

Choose a reason for hiding this comment

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

Nit: Your implementation considers anything more than log2(n) to be “large” (not ln or log10); log2 isn’t one of the bases abbreviated “log”.

Copy link
Copy Markdown
Author

@vanvoorden vanvoorden Dec 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@xwu Ahh… that's a good point!

TBH… I'm also open to hearing better ideas for how to compute this new threshold. Taking the binary logarithm guarantees our O(n log n) worst-case complexity but is also maybe more conservative than necessary. For an Array of one million elements this only gives us the first 19 elements before we sort the whole Array.

Adding one to the result of the binary logarithm gives us approximately log1.41 for the first 20 elements. We can keep going down that road and adding more… but I don't have a great answer how to analytically solve for what the optimal threshold should be that guarantees our O(n log n) worst-case complexity but keeps working on the "fast path" as much as possible.

the collection), we fall back to sorting the entire array. Realistically, this
means the worst case is actually `O(n log n)`.

Expand Down
8 changes: 4 additions & 4 deletions Sources/Algorithms/MinMax.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,9 +270,9 @@ extension Collection {
return []
}

// If we're attempting to prefix more than 10% of the collection, it's
// If we're attempting to prefix more than log n of the collection, it's
// faster to sort everything.
guard prefixCount < (self.count / 10) else {
guard prefixCount <= self.count._binaryLogarithm() else {
Copy link
Copy Markdown
Contributor

@xwu xwu Dec 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

You don't need underscored API for this:

Suggested change
guard prefixCount <= self.count._binaryLogarithm() else {
guard prefixCount <= (Int.bitWidth - self.count.leadingZeroBitCount) else {

return Array(try sorted(by: areInIncreasingOrder).prefix(prefixCount))
}

Expand Down Expand Up @@ -326,9 +326,9 @@ extension Collection {
return []
}

// If we're attempting to prefix more than 10% of the collection, it's
// If we're attempting to suffix more than log n of the collection, it's
// faster to sort everything.
guard suffixCount < (self.count / 10) else {
guard suffixCount <= self.count._binaryLogarithm() else {
return Array(try sorted(by: areInIncreasingOrder).suffix(suffixCount))
}

Expand Down