Skip to content

Commit

Permalink
Fix typo in heuristics of BracketedSort (#52494)
Browse files Browse the repository at this point in the history
The impact of this typo was a) massively decreased performance that was
b) predicted by heuristic dispatch, resulting in this algorithm not
being dispatched too. I introduced this typo in
187e8c2 after performing all the benchmarking and before merging.
  • Loading branch information
LilithHafner authored Dec 18, 2023
1 parent b57f8d1 commit 2ab4105
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 5 deletions.
2 changes: 1 addition & 1 deletion base/sort.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1262,7 +1262,7 @@ function _sort!(v::AbstractVector, a::BracketedSort, o::Ordering, kw)
k = cbrt(ln)
k2 = round(Int, k^2)
k2ln = k2/ln
offset = .15k2*top_set_bit(k2) # TODO for further optimization: tune this
offset = .15k*top_set_bit(k2) # TODO for further optimization: tune this
lo_signpost_i, hi_signpost_i =
(floor(Int, (tar - lo) * k2ln + lo + off) for (tar, off) in
((minimum(target), -offset), (maximum(target), offset)))
Expand Down
7 changes: 3 additions & 4 deletions test/sorting.jl
Original file line number Diff line number Diff line change
Expand Up @@ -721,10 +721,9 @@ end
for alg in safe_algs
@test sort(1:n, alg=alg, lt = (i,j) -> v[i]<=v[j]) == perm
end
# This could easily break with minor heuristic adjustments
# because partialsort is not even guaranteed to be stable:
@test partialsort(1:n, 172, lt = (i,j) -> v[i]<=v[j]) == perm[172]
@test partialsort(1:n, 315:415, lt = (i,j) -> v[i]<=v[j]) == perm[315:415]
# Broken by the introduction of BracketedSort in #52006 which is unstable
@test_broken partialsort(1:n, 172, lt = (i,j) -> v[i]<=v[j]) == perm[172]
@test_broken partialsort(1:n, 315:415, lt = (i,j) -> v[i]<=v[j]) == perm[315:415]

# lt can be very poorly behaved and sort will still permute its input in some way.
for alg in safe_algs
Expand Down

0 comments on commit 2ab4105

Please sign in to comment.