esp32/modesp32: Implement idf_task_stats(). #12732
Conversation
|
This looks pretty neat! The zephyr port already has I think this would be a good (optional) addition. |
1982e18 to
e2316bd
Compare
|
Thank you for the feedback! I've updated the branch to address them. One thing I'm not sure about is the rather large code snippet (compared to snippets elsewhere in the documentation) that implements the |
It could be added to |
e2316bd to
8ddc267
Compare
|
Updated to move the code snippet from the documentation to a separate function in |
8ddc267 to
5842015
Compare
5842015 to
65310c7
Compare
65310c7 to
fa7c95b
Compare
|
Note: The build failure is due to the linked PR for micropython-lib not being merged, and thus the docs are referring to a file that does not yet exist on the micropython-lib master branch. |
fa7c95b to
61cd01d
Compare
61cd01d to
aad79b4
Compare
| #if CONFIG_FREERTOS_VTASKLIST_INCLUDE_COREID | ||
| MP_OBJ_NEW_SMALL_INT(task_array[i].xCoreID), | ||
| #else | ||
| mp_const_none, |
There was a problem hiding this comment.
The utop module expects this entry to be an integer, so if CONFIG_FREERTOS_VTASKLIST_INCLUDE_COREID is disabled then utop crashes.
Maybe -1 can be used here instead of None, so that it's uniformly an integer?
(I did see that FreeRTOS returns -1 for some tasks' xCoreID, such as "Tmr Svc", so maybe a different sentinel integer like 255, or -255?)
There was a problem hiding this comment.
Note: -1 is the constant tskNO_AFFINITY, meaning the task is not pinned to one specific core. Mapping it in utop (perhaps listing it as any instead of -1?) could make that clearer for the casual user.
Partly because there's already different integer values to interpret I think explicitly using None makes it clearer that the value is not available at all. I've now explicitly documented that behaviour and fixed utop to correctly handle that. If you're getting an integer value, that's a valid value returned by the IDF, and when you get None you have not build with the required configuration options. Let me know what you think!
There was a problem hiding this comment.
OK, I think that's fine to keep it as None when the feature is not supported.
aad79b4 to
0b1d2b0
Compare
e91357e to
045d785
Compare
This adds a new function, `esp32.idf_task_info()`, that can be used to retrieve task statistics which is useful for diagnosing issues where some tasks are using up a lot of CPU time. It's best used in conjunction with the `utop` module from micropython-lib. Signed-off-by: Daniël van de Giessen <daniel@dvdgiessen.nl>
045d785 to
06d8c08
Compare
|
This works really well, thank you! FWIW, enabling it costs around 1k of flash. |
This adds a new function,
esp32.idf_task_stats(), that can be used to retrieve task statistics which is useful for diagnosing issues where some tasks are using up a lot of CPU time. For example, I originally found #8351 and #12728 using this function.An animated GIF is worth a thousand words:
(See also micropython/micropython-lib#822 which implements the Python function for this.)
By default the functionality is not enabled; it depends on a few extra option to be set. I have the following snippet in my
sdkconfig.boardfiles:If the required FreeRTOS functionality is available, the
idf_task_stats()function is automatically enabled.