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

add qsort as libc extension #28896

Closed
JonBruchim opened this issue Oct 4, 2020 · 5 comments · Fixed by #39980
Closed

add qsort as libc extension #28896

JonBruchim opened this issue Oct 4, 2020 · 5 comments · Fixed by #39980
Assignees
Labels
area: C Library C Standard Library area: Minimal libc Minimal C Standard Library Enhancement Changes/Updates/Additions to existing features

Comments

@JonBruchim
Copy link

qsort isn't implemented under stdlib

As we would like zephyr to have a small memory footprint,
adding this function to stdlib (as its in the standard c library) will increase the overhead
I would like to add this function under zephyr/lib/libc/minimal/

I would be glad to take this as my first issue in zephyr.

Thanks

@JonBruchim JonBruchim added the Enhancement Changes/Updates/Additions to existing features label Oct 4, 2020
@andrewboie
Copy link
Contributor

we use gc-sections so I don't see any particular issue with adding to minimal libc since if it is unused, its code won't be included in the binary.

@JonBruchim
Copy link
Author

JonBruchim commented Nov 3, 2020

Ive actually made a PR adding musl libc implementation
but I've closed it after thinking again about the implementations made in subsys/shell/shell_module/src/qsort.c
should we remove them?
Ill open the PR again

@cfriedt
Copy link
Member

cfriedt commented Sep 29, 2021

Also covered in #31107

@cfriedt
Copy link
Member

cfriedt commented Sep 29, 2021

Copied from #29804

I have some reservations about just copying from another project, although I haven't looked at the implementation.

  • qsort does not actually specify which sorting algorithm should be used under the hood, which leaves a lot of possibilities open for the implementation. I've read that musl has implemented Smoothsort
  • if we add a sorting algorithm to the minimal libc, let's add one that has decent worst-case performance.
  • if we add a sorting algorithm to the minimal libc, let's add one that has all of the other random features that we could want, such as

I mistakenly opened #31107 for this same topic since I've been carrying qsort.c in my greybus module for a while (from the BSD C library). However, due to the aforementioned concerns, I never bothered to make a PR.

@JonBruchim - would you be able to provide some analysis of this vs possibly other algorithms?

Just at a quick glance, Block Sort looks promising too.

https://en.wikipedia.org/wiki/Block_sort

  • O(N) best case
  • O(NlogN) worst / average case
  • O(1) space
  • Stable
  • In-place
    I haven't looked to see if it can be implemented easily though, or what kind of stack space it might require. That's also obviously a concern.

I guess the only slight advantage that Block Sort might have is that it's stable. Smoothsort looks to have a reasonably simple implementation though.

@cfriedt
Copy link
Member

cfriedt commented Nov 9, 2021

Discussed today in the release meeting, this change in #39980 may be backported to 2.7 because it:

It satisfies several requirements of the Minimal LibC

  • minimal amount of maintenance required
  • small code footprint
  • decent performance

Lastly, it satisfies several criteria for Safety Critical

  • reasonable asymptotic bounds:
    • time: O(log(n)) in best, average, and worst cases
    • space: O(1)
  • iterative solution as opposed to recursive
  • operates in-place

cfriedt added a commit to cfriedt/zephyr that referenced this issue Nov 10, 2021
This change implements qsort() for the minimal libc via Heapsort.

Heapsort time complexity is O(n log(n)) in the best, average,
and worst cases. It is O(1) in space complexity (i.e. sorts
in-place) and is iterative rather than recursive. Heapsort is
not stable (i.e. does not preserve order of identical elements).

On cortex-m0, this implementation occupies ~240 bytes.

Fixes zephyrproject-rtos#28896

Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
cfriedt added a commit that referenced this issue Nov 10, 2021
This change implements qsort() for the minimal libc via Heapsort.

Heapsort time complexity is O(n log(n)) in the best, average,
and worst cases. It is O(1) in space complexity (i.e. sorts
in-place) and is iterative rather than recursive. Heapsort is
not stable (i.e. does not preserve order of identical elements).

On cortex-m0, this implementation occupies ~240 bytes.

Fixes #28896

Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
zephyrbot pushed a commit that referenced this issue Nov 10, 2021
This change implements qsort() for the minimal libc via Heapsort.

Heapsort time complexity is O(n log(n)) in the best, average,
and worst cases. It is O(1) in space complexity (i.e. sorts
in-place) and is iterative rather than recursive. Heapsort is
not stable (i.e. does not preserve order of identical elements).

On cortex-m0, this implementation occupies ~240 bytes.

Fixes #28896

Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
cfriedt added a commit that referenced this issue Dec 8, 2021
This change implements qsort() for the minimal libc via Heapsort.

Heapsort time complexity is O(n log(n)) in the best, average,
and worst cases. It is O(1) in space complexity (i.e. sorts
in-place) and is iterative rather than recursive. Heapsort is
not stable (i.e. does not preserve order of identical elements).

On cortex-m0, this implementation occupies ~240 bytes.

Fixes #28896

Signed-off-by: Christopher Friedt <chrisfriedt@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: C Library C Standard Library area: Minimal libc Minimal C Standard Library Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants