Conversation
`partition` places values equal to the pivot on the low side of the partition point, so on a dataset where a single value is very common the range being searched can shrink by only one element per iteration. That makes `selectInPlace` quadratic: computing the median of 200,000 equal values took about 5.9 seconds. Rather than replace the partition wholesale, which measurably slowed down duplicate-free datasets, keep the existing partition and give it a budget of iterations comfortably larger than well-behaved data needs. Only once that budget is exhausted do we switch to a three-way partition, which gathers every value equal to the pivot into one run so the whole run can be excluded at once. Median of 200,000 values, before -> after: all values equal 5875.76 ms -> 2.65 ms 5 distinct values 208.67 ms -> 1.57 ms 100 distinct values 2.37 ms -> 1.51 ms gaussian (all distinct) 1.52 ms -> 1.61 ms The existing tests all use Gaussian data, in which duplicates never occur, so this adds coverage for duplicate-heavy datasets too. Fixes google#3789
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes issue no #3789.
Sending this because of your comment on the issue about maybe having a look if the fix were easy enough to understand. Happy to close it if you'd rather not take this on.
Problem
partitionplaces values equal to the pivot on the low side of the partition point. On a dataset where one value is very common, the range being searched shrinks by only one element per iteration, soselectInPlacebecomes quadratic.The median of 200,000 equal values takes about 5.9 seconds.
Why not simply replace the partition
I tried that first, three ways: a Dutch-national-flag partition, Bentley-McIlroy ,and a Hoare partition that splits equal values across both sides. All three fix the duplicate case, and all three cost 20-25% on duplicate-free input, measured over 400 independent datasets per configuration. The existing countdown loop optimizes better than any partition with data-dependent loop bounds, so the cost looks inherent rather than an artefact of my implementations.
So this keeps the existing partition for the common case and gives it a budget of iterations comfortably larger than well-behaved data needs. Only once that budget is exhausted do we switch to a three-way partition, which gathers every value equal to the pivot into a single run so the run can be excluded in one step.
Duplicate-free input never reaches the fallback and pays only one decrement and one comparison per partition step.
Numbers
Median of 200,000 values, end to end through the public API:
dataset : before --> after
all values equal : 5875.76 ms --> 2.65 ms
5 distinct values : 208.67 ms --> 1.57 ms
100 distinct values : 2.37 ms --> 1.51 ms
gaussian (all distinct) : 1.52 ms --> 1.61 ms
Tests
Every existing
Quantilestest dataset is Gaussian, so duplicates never arise and this case was uncovered. The new tests are sized so they would take minutes rather than seconds against the old code.Notes