Skip to content

Optimize Dataset sort() - #431

Open
andrewdalpino wants to merge 5 commits into
3.0from
optimize-dataset-sort
Open

Optimize Dataset sort()#431
andrewdalpino wants to merge 5 commits into
3.0from
optimize-dataset-sort

Conversation

@andrewdalpino

@andrewdalpino andrewdalpino commented Aug 22, 2026

Copy link
Copy Markdown
Member

Dataset::sort() is an O(n²) bubble sort — confirmed at src/Datasets/Dataset.php:441-459. Switch to native usort() (Timsort, O(n log n), C-implemented). 10–100× for n > ~1000.

n Before (bubble, O(n²)) After (usort, O(n log n)) Speedup
100 0.765 ms ±1.2% 0.384 ms ±1.3% 2.0×
1,000 44.343 ms ±0.7% 2.961 ms ±1.0% 15.0×
2,500 195.562 ms ±1.0% 8.288 ms ±1.2% 23.6×
5,000 759.693 ms ±0.7% 17.776 ms ±1.2% 42.7×

Targetting 3.0 branch as this changes the operation from an exact sort to an unstable sort.

@andrewdalpino
andrewdalpino requested review from a team and a lite review from Copilot August 22, 2026 17:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR replaces Dataset::sort()’s O(n²) bubble sort with PHP’s native usort()-based approach to improve sorting performance on large datasets, and updates the dataset sorting benchmark to cover multiple dataset sizes.

Changes:

  • Replaced the manual bubble sort in Rubix\ML\Datasets\Dataset::sort() with a usort() comparator wrapper.
  • Updated SortingBench to benchmark multiple dataset sizes via PhpBench parameter providers.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/Datasets/Dataset.php Switches dataset sorting implementation from bubble sort to usort() for O(n log n) behavior.
benchmarks/Datasets/SortingBench.php Expands the benchmark to run sorting across multiple dataset sizes using param providers.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread benchmarks/Datasets/SortingBench.php
Comment thread src/Datasets/Dataset.php
@andrewdalpino
andrewdalpino changed the base branch from master to 3.0 August 22, 2026 17:34
@andrewdalpino andrewdalpino added the optimization Make something perform faster label Aug 23, 2026
@apphp

apphp commented Aug 23, 2026

Copy link
Copy Markdown

@andrewdalpino do we need this file in repo: runtime/test-results ?
What do you thing if we add it to .gitignore?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

optimization Make something perform faster

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants