-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add Skip List implementation and unit test for improved scheduler determinism #1287
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
Conversation
test_skip_list.exe
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this binary file included?
test_empty_file.txt
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this empty file included?
This appears to be a standalone implementation that is not utilized by the kernel. If you intend to use it in an application, you should be able to do so without issue. However, given that it is not integrated into the kernel's functionality, I don't believe it should be merged into the kernel codebase. |
skip_list.c
Outdated
|
||
static int xRandomLevel(void) { | ||
int level = 1; | ||
while ((rand() & 0x1) && level < SKIP_LIST_MAX_LEVEL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't a better way would be to level = rand()%SKIP_LIST_MAX_LEVEL
? Am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't a better way would be to
level = rand()%SKIP_LIST_MAX_LEVEL
? Am I missing something?
The idea here isn't just to "pick a random number" — it's to generate levels following a geometric distribution, where each higher level is about half as likely as the previous one. This gives us a nice layered effect in the skip list: tons of nodes at level 1, fewer at level 2, even fewer at level 3, and so on.
level = rand() % SKIP_LIST_MAX_LEVEL;
…we’d be assigning levels with uniform probability. That would mean you might end up with too many high-level nodes, which breaks the performance benefits. Worst case, it starts behaving more like a flat list than a skip list.
test_skip_list.c
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I appreciate adding a test for your implementation but consider adding any unit tests here. This would take another PR.
…on to uxListRemove
|
Add Skip List implementation and unit test for improved scheduler determinism
Description
This PR introduces a Skip List data structure to the FreeRTOS kernel, providing O(log n) insertion and search for sorted lists. It includes:
This enhancement is intended to improve temporal determinism for scheduler operations that previously relied on O(n) list traversal.
Test Steps
Checklist:
Related Issue
N/A
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.