poll: Minimize overhead when polling single event #1396
Conversation
kernel/poll.c
Outdated
| @@ -197,6 +231,10 @@ int k_poll(struct k_poll_event *events, int num_events, s32_t timeout) | |||
| int last_registered = -1, rc; | |||
| unsigned int key; | |||
|
|
|||
| if (num_events == 1) { | |||
| return poll_1(events, timeout); | |||
There was a problem hiding this comment.
why not just a goto to a label on line 241? I am not sure introducing poll_1() optimize that much the path.
kernel/poll.c
Outdated
| @@ -246,7 +246,7 @@ int k_poll(struct k_poll_event *events, int num_events, s32_t timeout) | |||
|
|
|||
| _pend_current_thread(&wait_q, timeout); | |||
|
|
|||
| int swap_rc = _Swap(key); | |||
| rc = _Swap(key); | |||
There was a problem hiding this comment.
Why? This surely doesn't affect the generated code at all (and if so only spuriously by e.g. changing register assignments), and honestly IMHO it was clearer with one specific/named register per use. Mutable variables are a bad thing when not needed.
There was a problem hiding this comment.
I always thought this would require another register, isn't that a common practice not to introduce new variable if you can reuse them. Perhaps this depends on the compiler and if it can detect there is no overlapping of rc and swap_rc, but depending on the compiler to do the right thing sometimes can backfire.
kernel/poll.c
Outdated
| @@ -197,6 +231,10 @@ int k_poll(struct k_poll_event *events, int num_events, s32_t timeout) | |||
| int last_registered = -1, rc; | |||
| unsigned int key; | |||
|
|
|||
| if (num_events == 1) { | |||
| return poll_1(events, timeout); | |||
There was a problem hiding this comment.
Have you benchmarked this? The net effect here is a fairly significant increase in code size, and what you gain is AFAICT that the loop exit test in the needs to be executed only once (i.e. if(num_events == 1) is basically the same test) instead of twice, and the final irq_unlock/lock pair gets to be elided.
That's just a tiny handful of cycles. I'm sure it's faster, but I doubt it's much faster and I'd say we really need to see numbers before adding this.
There was a problem hiding this comment.
My understanding is that this patch was an attempt to address https://jira.zephyrproject.org/browse/ZEP-2593 , but per my comment above, it doesn't help.
There was a problem hiding this comment.
Well, I was coming from the point that IRQ should be locked as little as possible since this is now used for by buffer polls this is a very heavy dependency on this code path.
…ect-rtos#1396) Configure will now call the helper method to set both modes instead of writing directory to the register, this fixed an issue where app cannot reconfigure classification mode once it is set. Fixes zephyrproject-rtos#1254 Signed-off-by: Jimmy Huang <[email protected]>
It is now possible to poll event if there is another thread polling. Signed-off-by: Luiz Augusto von Dentz <[email protected]>
In case K_POLL_STATE_NOT_READY is set the return will be set to -EINTR indicating that the poll was interrupted. Fixes zephyrproject-rtos#5026 Signed-off-by: Luiz Augusto von Dentz <[email protected]>
This attempts to minimize as much as possible when polling a single event which is what k_queue_get does when CONFIG_POLL is enabled.
Jira: 2593