Move WiFi queue operations to C-side ring buffer#42
Conversation
| return NULL; | ||
| } | ||
|
|
||
| void *espradio_generic_queue_create(uint32_t queue_len, uint32_t item_size) { |
There was a problem hiding this comment.
This function returns NULL silently if all 8 slots are taken which would probably cause the blob to crash in a non-obvious way.
The previous code had no limit, due to using a hash map. You should probably log on exhaustion under ESPRADIO_OSI_DEBUG, and ideally verify empirically how many queues the blob actually creates.
There was a problem hiding this comment.
Fixed.
I removed the fixed ESPRADIO_QUEUE_MAX slot array and replaced it with dynamically allocated queue records linked from s_queues_head, so queue creation is no longer capped at 8 entries.
I verified empirically on my current startup/runtime path that the blob creates only one queue. I do not think that fully covers every possible WiFi/runtime path though, so I kept the implementation uncapped instead of relying on a larger fixed limit.
| for (unsigned i = 0; i < ESPRADIO_QUEUE_MAX; i++) { | ||
| espradio_queue_t *q = &s_queues[i]; | ||
| if (!q->used) continue; | ||
| if (ptr == q || ptr == &q->handle) return q; |
There was a problem hiding this comment.
I think you can just check ptr == q here since handle is the first field in the struct.
| if (!forever && espradio_time_us_now() >= deadline) return 0; | ||
| uint64_t target = espradio_time_us_now() + 1000ULL; | ||
| espradio_task_yield_go(); | ||
| while (espradio_time_us_now() < target) { |
There was a problem hiding this comment.
After yielding, this spins for 1ms burning CPU. The previous code called safeGosched() without the busy wait.
How about dropping the spin and calling espradio_task_yield_go() repeatedly, matching the old behavior?
|
Thank you for working on this @i-melnichenko I made a few suggestions. I tested and it seems to work, although I had to increase the default arena size on esp32c3 to get it to do so. |
|
Thanks for reviewing and testing! Good point on the arena size. Earlier in this PR I introduced the fixed I verified on my current startup/runtime path that the blob creates only one queue, but that may not cover every WiFi path. If |
|
Thank you for the improvements and the requested changes @i-melnichenko now squash/merging. |
2 participants
Moves the WiFi queue implementation from Go callbacks to a C-side arena-backed ring buffer.
Sustained WiFi traffic after Enable() + Start() caused continuous Go heap growth due to repeated C-to-Go queue callback transitions during queue operations. Keeping queue create/send/recv/delete/len entirely on the C side removes that allocation churn from the Go runtime.
Observed after Enable() + Start():