Skip to content

memory profiling: heap memory profiling at runtime #6525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lgirdwood merged 1 commit into from
Nov 16, 2022
Merged

memory profiling: heap memory profiling at runtime #6525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/perf_overlay.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
CONFIG_PERFORMANCE_COUNTERS=y
CONFIG_SYS_HEAP_RUNTIME_STATS=y
Copy link
Member

@lgirdwood lgirdwood Nov 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be a debug overlay @kv2019i ?

Copy link
Contributor Author

@btian1 btian1 Nov 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense if put is as default debug overlay, since its functionality is for stack overflow detection.

Copy link
Member

@lgirdwood lgirdwood Nov 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry not following how the heap state helps with stack overflow detection, is this a different Zephyr option ?

Copy link
Contributor Author

@btian1 btian1 Nov 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they are separate functionality, no relation between heap and stack overflow detection.
CONFIG_SYS_HEAP_RUNTIME_STATS is used to collect heap allocated/free space, total is around 1.4MB.

CONFIG_STACK_SENTINEL this is used to enable stack overflow detection by check a magic number in the stack bottom, after enable this, once stack overflow happened, Zephyr logging will print "Stack overflow" with other information, so move it to debug build config is also good, please let me know, if you want to move it.

Copy link
Contributor Author

@btian1 btian1 Nov 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#6530
this already added, @kv2019i , do you think I need remove here?
once remove, I need add -d option for perf build.

Copy link
Member

@lgirdwood lgirdwood Nov 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@btian1 see above, we need these 2 Kconfigs to allow the developers to tune the output depending on what they are debugging.

Copy link
Contributor Author

@btian1 btian1 Nov 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lgirdwood , I did not find in current SOF code base, add it from new and then add code to make these config work?

I also want to add this at the beginning, however, due to wrapped too much layer, I did not try with this.
It may need introduce new parameters for each function and code change may be big, any better ideas?

rmalloc/rzalloc...., need add pipeline and component info for each memory alloc and free.

Copy link
Collaborator

@lyakh lyakh Nov 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

config SOF_HEAP_RUNTIME_ALLOC

@lgirdwood Maybe add "DEBUG" or "PRINT" somewhere? Otherwise this option looks to me like it enables heap allocations and without it rmalloc() won't work. Same for SOF_HEAP_RUNTIME_PIPELINE perhaps

Copy link
Collaborator

@lyakh lyakh Nov 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also suspect that those options would have quite a large run-time / logging impact, so I'd really rather have them off by default and only enable them for debugging. The former would supposedly "only" print one line for each allocations, but we allocate quite a lot. We have many instances where we initialise a single functionality but use several allocations for it. Hopefully we don't allocate any memory for each .copy() / .process() iteration, but even when one pipeline is running and another one processes IPCs which will print excessive additional information for memory allocations - that can affect performance of the running pipeline.
Same holds for heap state dumps for each pipeline creation and destruction. Especially since we now have multiple pipelines for most or all streams. I know we have those dumps with XTOS and I find them quite intrusive and excessively polluting logs too (see #4744 )

Copy link
Contributor Author

@btian1 btian1 Nov 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RanderWang , Rander today introduced that close fw alloc memory based on pipeline, this is much more easy to do data collection and analysis.

For SOF, allocation based on rmalloc, any code call this module can get a buffer with specific size, the difficult part is we have to add all called place with pipe and component information.

13 changes: 13 additions & 0 deletions zephyr/wrapper.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@
#include <zephyr/arch/xtensa/cache.h>
#endif

#if CONFIG_SYS_HEAP_RUNTIME_STATS
#include <zephyr/sys/sys_heap.h>
#endif

LOG_MODULE_REGISTER(zephyr, CONFIG_SOF_LOG_LEVEL);

extern K_KERNEL_STACK_ARRAY_DEFINE(z_interrupt_stacks, CONFIG_MP_NUM_CPUS,
Expand Down Expand Up @@ -108,11 +112,20 @@ static void *heap_alloc_aligned(struct k_heap *h, size_t min_align, size_t bytes
{
k_spinlock_key_t key;
void *ret;
#if CONFIG_SYS_HEAP_RUNTIME_STATS
struct sys_memory_stats stats;
#endif

key = k_spin_lock(&h->lock);
ret = sys_heap_aligned_alloc(&h->heap, min_align, bytes);
k_spin_unlock(&h->lock, key);

#if CONFIG_SYS_HEAP_RUNTIME_STATS
sys_heap_runtime_stats_get(&h->heap, &stats);
tr_info(&zephyr_tr, "heap allocated: %u free: %u max allocated: %u",
stats.allocated_bytes, stats.free_bytes, stats.max_allocated_bytes);
#endif

return ret;
}

Expand Down