core: make clist singly-linked #4923
Conversation
|
Nice! |
|
I like! |
| @@ -16,7 +16,8 @@ | |||
|
|
|||
| #define TEST_CLIST_LEN (8) | |||
|
|
|||
| static clist_node_t tests_clist_buf[TEST_CLIST_LEN]; | |||
| static list_node_t tests_clist_buf[TEST_CLIST_LEN]; | |||
There was a problem hiding this comment.
Here and below: the datatype is called list now so the variable, function and file names should reflect that.
There was a problem hiding this comment.
Even if clist_*() works with list_node_t objects?
There was a problem hiding this comment.
But doesn't clist stand for circular list? (or is it that still)?
There was a problem hiding this comment.
Yes. I shared the list node definition between the two header files. Now I copied the definition, do you prefer it that way?
There was a problem hiding this comment.
Why copy when you can
typedef list_t clist_t;There was a problem hiding this comment.
typedef'ed it is now.
6a376dd to
62504f3
Compare
|
|
Why does this depend on #4557? This data structure should be independent of that change, right? |
|
Why can't you take the list.h introduction over here and make #4557 dependent on this PR. That seems to be the more logical dependency graph to me. |
|
Dependency not yet merged. Postponed. |
|
Sad. |
|
Agreed, but currently I don't see realistic chances for #4557 to get merged until tomorrow. Do you disagree? |
|
Can't review it myself... |
|
You can kick your reviewers. ;) |
cf058ed to
b308448
Compare
| * | ||
| * Each list is represented as a "clist_node_t". It's only member, the "next" | ||
| * pointer, points to the last entry in the list, whose "next" pointer points to | ||
| * the first entry. |
There was a problem hiding this comment.
Something reads weird here, how about
... . It's only member, the "next" pointer, points to the next entry in the list. The last element's "next" pointer points to the first entry.

There was a problem hiding this comment.
missread the sentence, never mind...
|
Needs rebase. |
b308448 to
0415ff9
Compare
|
|
Code looks good, works as expected (on 'native' and some ARM boards), code size is smaller than before -> ACK! |
|
code looks sensible and changes make sense. ACK |
0415ff9 to
1bac3f3
Compare
clist was doubly linked in order to enable removal of threads from a run queue in constant time.
Today I realized that only the currently running thread can remove itself from a running queue (by blocking for a msg, mutex, sleeping, ...), and a singly linked but circular list offers O(1) removal of the head element.
So I changed clist accordingly.
This carves off 4 byte of tcb and increases context switch
timeperformance by ~4%.Waiting for #4557.