misc/dlist: Swap insertion API for a faster one

The sys_dlist_insert_*() functions had a behavior where a NULL
argument for the insertion position to sys_dlist_insert_after/before()
was interpreted as "the end of the list".  We never used that
convention (except in one spot internal to dlist.h which was not
itself used anywhere), and of course already have an API for appending
and prepending to a list.

In practice this was a performance disaster.  The NULL check is
virtually never provable statically by the compiler, so that test and
branch is present always.  And worse, the check and call to another
function was pushing this beyond the complexity limit for gcc to
inline a function (at -Os optimization anyway), forcing us to use
function calls for what should be a ~8 instruction sequence.  The
upshot is that dlist insertions were 2-3x slower than they needed to
be.

Deprecate these older APIs and introduce a new sys_dlist_insert() call
which can be much better optimized.

Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
This commit is contained in:
Andy Ross 2019-01-28 09:35:27 -08:00 committed by Anas Nashif
parent aa3c9b7f1e
commit eda4c027da
5 changed files with 25 additions and 33 deletions

View File

@ -22,6 +22,7 @@
#include <stddef.h>
#include <stdbool.h>
#include <toolchain.h>
#ifdef __cplusplus
extern "C" {
@ -429,19 +430,22 @@ static inline void sys_dlist_prepend(sys_dlist_t *list, sys_dnode_t *node)
}
/**
* @brief insert node after a node
* @brief Insert a node into a list
*
* Insert a node after a specified node in a list.
* This and other sys_dlist_*() functions are not thread safe.
* Insert a node before a specified node in a dlist.
*
* @param list the doubly-linked list to operate on
* @param insert_point the insert point in the list: if NULL, insert at head
* @param node the element to append
*
* @return N/A
* @param successor the position before which "node" will be inserted
* @param node the element to insert
*/
static inline void sys_dlist_insert(sys_dnode_t *successor, sys_dnode_t *node)
{
node->prev = successor->prev;
node->next = successor;
successor->prev->next = node;
successor->prev = node;
}
static inline void sys_dlist_insert_after(sys_dlist_t *list,
static inline void __deprecated sys_dlist_insert_after(sys_dlist_t *list,
sys_dnode_t *insert_point, sys_dnode_t *node)
{
if (insert_point == NULL) {
@ -454,20 +458,7 @@ static inline void sys_dlist_insert_after(sys_dlist_t *list,
}
}
/**
* @brief insert node before a node
*
* Insert a node before a specified node in a list.
* This and other sys_dlist_*() functions are not thread safe.
*
* @param list the doubly-linked list to operate on
* @param insert_point the insert point in the list: if NULL, insert at tail
* @param node the element to insert
*
* @return N/A
*/
static inline void sys_dlist_insert_before(sys_dlist_t *list,
static inline void __deprecated sys_dlist_insert_before(sys_dlist_t *list,
sys_dnode_t *insert_point, sys_dnode_t *node)
{
if (insert_point == NULL) {
@ -508,7 +499,11 @@ static inline void sys_dlist_insert_at(sys_dlist_t *list, sys_dnode_t *node,
while (pos && !cond(pos, data)) {
pos = sys_dlist_peek_next(list, pos);
}
sys_dlist_insert_before(list, pos, node);
if (pos != NULL) {
sys_dlist_insert(pos, node);
} else {
sys_dlist_append(list, node);
}
}
}

View File

@ -91,8 +91,7 @@ static inline void add_event(sys_dlist_t *events, struct k_poll_event *event,
SYS_DLIST_FOR_EACH_CONTAINER(events, pending, _node) {
if (_is_t1_higher_prio_than_t2(poller->thread,
pending->poller->thread)) {
sys_dlist_insert_before(events, &pending->_node,
&event->_node);
sys_dlist_insert(&pending->_node, &event->_node);
return;
}
}

View File

@ -573,8 +573,8 @@ void _priq_dumb_add(sys_dlist_t *pq, struct k_thread *thread)
SYS_DLIST_FOR_EACH_CONTAINER(pq, t, base.qnode_dlist) {
if (_is_t1_higher_prio_than_t2(thread, t)) {
sys_dlist_insert_before(pq, &t->base.qnode_dlist,
&thread->base.qnode_dlist);
sys_dlist_insert(&t->base.qnode_dlist,
&thread->base.qnode_dlist);
return;
}
}

View File

@ -87,8 +87,7 @@ void _add_timeout(struct _timeout *to, _timeout_func_t fn, s32_t ticks)
if (t->dticks > to->dticks) {
t->dticks -= to->dticks;
sys_dlist_insert_before(&timeout_list,
&t->node, &to->node);
sys_dlist_insert(&t->node, &to->node);
break;
}
to->dticks -= t->dticks;

View File

@ -170,7 +170,7 @@ static inline bool verify_tail_head(sys_dlist_t *list,
* @brief Verify doubly linked list funtionalities
*
* @see sys_dlist_append(), sys_dlist_remove(), sys_dlist_prepend(),
* sys_dlist_remove(), sys_dlist_insert_after(), sys_dlist_peek_next()
* sys_dlist_remove(), sys_dlist_insert(), sys_dlist_peek_next()
* SYS_DLIST_ITERATE_FROM_NODE()
*/
void test_dlist(void)
@ -238,8 +238,7 @@ void test_dlist(void)
"test_list node links are wrong");
/* Inserting node 4 after node 2 */
sys_dlist_insert_after(&test_list, &test_node_2.node,
&test_node_4.node);
sys_dlist_insert(test_node_2.node.next, &test_node_4.node);
zassert_true((verify_tail_head(&test_list, &test_node_2.node,
&test_node_3.node, false)),