Skip to content
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.

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

Unstable sorting in standard library should be faster #15388

Closed
matklad opened this issue Apr 21, 2023 · 8 comments · Fixed by #15412
Closed

Unstable sorting in standard library should be faster #15388

matklad opened this issue Apr 21, 2023 · 8 comments · Fixed by #15412
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. optimization standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@matklad
Copy link
Contributor

matklad commented Apr 21, 2023

Zig Version

0.11.0-dev.2685+fac120bc3

Steps to Reproduce and Observed Behavior

Reproducible benchmark here:

https://github.com/matklad/benchmarks/tree/2b07628b8bbf5c9258c807aa8a221a0797f247b9/sort

The interesting bits:

Rust + unstable:
424.10ms

Rust + stable:
765.45ms

Zig
1.147s

By itself, the number is not surprising. Zig's sort is both stable and in place, while Rust's stable sort allocates.

However, while stable sort is a good default choice, Zig strives for optimality, so the user should be able to choose better performance if stability is not a requirement.

Expected Behavior

std.sort.sortUnstable exists and it is as fast as state of the art. I noticed that

https://github.com/alichraghi/zort/blob/67493f9412c65526fd0a5db447b52d53e282a934/src/pdqsort.zig

actually is a tiny bit faster than even Rust on my benchmark, so perhaps we can add that to stdlib? cc @alichraghi

@matklad matklad added the bug Observed behavior contradicts documented or intended behavior label Apr 21, 2023
@matklad
Copy link
Contributor Author

matklad commented Apr 21, 2023

Also, if you have time @orlp, it would be great to pick your brain here!

My current understanding is that https://github.com/orlp/glidesort is the current state of the art, if you have some extra memory to spare. But for cases where you don't want/can't allocate, pdq is still the champion, right?

@voroskoi
Copy link
Contributor

Well, https://github.com/scandum/crumsort could be interesting, but I can not understand quadsort code, so I gave up my porting efforts.

@andrewrk andrewrk added contributor friendly This issue is limited in scope and/or knowledge of Zig internals. optimization standard library This issue involves writing Zig code for the standard library. and removed bug Observed behavior contradicts documented or intended behavior labels May 23, 2023
@andrewrk andrewrk added this to the 0.11.0 milestone May 23, 2023
@andrewrk
Copy link
Member

andrewrk commented May 24, 2023

Keeping this open since std lib hash map sorting still uses insertion sort:

zig/lib/std/mem.zig

Lines 587 to 591 in 3db3cf7

/// TODO: currently this just calls `insertionSortContext`. The block sort implementation
/// in this file needs to be adapted to use the sort context.
pub fn sortContext(a: usize, b: usize, context: anytype) void {
std.sort.insertionContext(a, b, context);
}

@andrewrk andrewrk reopened this May 24, 2023
@andrewrk andrewrk modified the milestones: 0.11.0, 0.12.0 May 24, 2023
@alichraghi
Copy link
Contributor

there is #11117

@andrewrk
Copy link
Member

Ah. But, isn't rust's stable sort faster than zig's, as mentioned in the issue description?

@alichraghi
Copy link
Contributor

yes. rust implements timsort as their main stable sort

@matklad
Copy link
Contributor Author

matklad commented May 24, 2023

Rust’s stable sort allocates, so it’s not immediately obvious that zig stable sort can be as fast, given that it doesn’t.

@andrewrk andrewrk changed the title Sorting in standard library should be faster Unstable sorting in standard library should be faster May 24, 2023
@andrewrk
Copy link
Member

Ah I see. With that in mind, nice work, @alichraghi!

@andrewrk andrewrk modified the milestones: 0.12.0, 0.11.0 May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. optimization standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants