[trace]: Add task naming capability to tracing infrastructure #4147
Conversation
embassy-executor/src/raw/trace.rs
Outdated
| /// | ||
| /// Contains the task's unique identifier and optional name. | ||
| #[derive(Clone)] | ||
| pub struct TrackedTask { |
There was a problem hiding this comment.
You'll have a better chance with this PR if you store this data in the task headers, instead of wasting 12-16K RAM.
There was a problem hiding this comment.
@bugadani Thanks for the review. I've updated my implementation to move the task information to TaskHeader and have used a LinkedList instead to keep track of all active tasks.
When rtos-trace is not enabled, spawn_named will use spawn instead
kat-perez
changed the title
embassy-executor/src/spawner.rs
Outdated
| /// | ||
| /// # Returns | ||
| /// Result indicating whether the spawn was successful | ||
| #[cfg(feature = "rtos-trace")] |
There was a problem hiding this comment.
Seems like this should be feature = "trace" ?
embassy-executor/src/spawner.rs
Outdated
|
|
||
| /// When rtos-trace is disabled, spawn_named falls back to regular spawn. | ||
| /// This maintains API compatibility while optimizing out the name parameter. | ||
| #[cfg(not(feature = "rtos-trace"))] |
There was a problem hiding this comment.
Seems like this should be feature = "trace" ?
embassy-executor/src/raw/trace.rs
Outdated
| pub fn get_all_active_tasks() -> impl Iterator<Item = TaskRef> + 'static { | ||
| struct TaskIterator<'a> { | ||
| tracker: &'a TaskTracker, | ||
| current: *mut TaskHeader, | ||
| } | ||
|
|
||
| impl<'a> Iterator for TaskIterator<'a> { | ||
| type Item = TaskRef; | ||
|
|
||
| fn next(&mut self) -> Option<Self::Item> { | ||
| if self.current.is_null() { | ||
| return None; | ||
| } | ||
|
|
||
| let task = unsafe { TaskRef::from_ptr(self.current) }; | ||
| self.current = unsafe { (*self.current).all_tasks_next.load(Ordering::Acquire) }; | ||
|
|
||
| Some(task) | ||
| } | ||
| } | ||
|
|
||
| TaskIterator { | ||
| tracker: &TASK_TRACKER, | ||
| current: TASK_TRACKER.head.load(Ordering::Acquire), | ||
| } | ||
| } | ||
|
|
||
| /// Get all active tasks, filtered by a predicate function | ||
| #[cfg(feature = "trace")] | ||
| pub fn filter_active_tasks<F>(predicate: F) -> impl Iterator<Item = TaskRef> + 'static | ||
| where | ||
| F: Fn(&TaskRef) -> bool + 'static, | ||
| { | ||
| get_all_active_tasks().filter(move |task| predicate(task)) | ||
| } | ||
|
|
||
| /// Count the number of active tasks | ||
| #[cfg(feature = "trace")] | ||
| pub fn count_active_tasks() -> usize { | ||
| let mut count = 0; | ||
| TASK_TRACKER.for_each(|_| count += 1); | ||
| count | ||
| } | ||
|
|
||
| /// Perform an action on each active task | ||
| #[cfg(feature = "trace")] | ||
| pub fn with_all_active_tasks<F>(f: F) | ||
| where | ||
| F: FnMut(TaskRef), | ||
| { | ||
| TASK_TRACKER.for_each(f); | ||
| } | ||
|
|
||
| /// Get tasks by name | ||
| #[cfg(feature = "trace")] | ||
| pub fn get_tasks_by_name(name: &'static str) -> impl Iterator<Item = TaskRef> + 'static { | ||
| filter_active_tasks(move |task| task.name() == Some(name)) | ||
| } | ||
|
|
||
| /// Get tasks by ID | ||
| #[cfg(feature = "trace")] | ||
| pub fn get_task_by_id(id: u32) -> Option<TaskRef> { | ||
| filter_active_tasks(move |task| task.id() == id).next() | ||
| } |
There was a problem hiding this comment.
Does these need to be public, or can they be internal? I'm a bit confused by the 'scattered' API, which makes it a bit hard to verify that it's all safe.
There was a problem hiding this comment.
They can be internal. I will change them to private and update the unused public functions instead.
|
I do want to take a look at this, if we end up merging #4035, it might make sense to re-use cordyceps for the intrusive linked list of (named) tasks |
Yeah, I can define a trait instead in trace.rs which extends Spawner since the spawn_named function is used exclusively for SystemView at the moment |
kat-perez
changed the title
|
@lulf @jamesmunns Updated PR to move more of the functionality into trace.rs, please review. Thanks! |
This change adds a TaskTracker to the tracing infrastructure. The name and ID of a task are now stored in TaskHeader and a new function called
spawn_namedis added to assign a name to the task for viewing in SystemView.