[v5.1] [Heap Trace Standalone] support external memory for hash map (IDFGH-9845)

[chipweinberger]

@SoucheSouche

Thanks for all your work on #10793.

I noticed that the hash map no longer supports external memory. I wont be able to use the hash map in my project without it as I don't have very much spare internal RAM. =(

Can you please add support for external ram?

    config HEAP_TRACE_HASH_MAP
        bool "Use hash map mechanism to access heap trace records"
        depends on HEAP_TRACING_STANDALONE
        default n
        help
            ....
            External ram can be used for this map if both CONFIG_SPIRAM_ALLOW_BSS_SEG_EXTERNAL_MEMORY 
            and HEAP_TRACE_HASH_MAP_USE_EXTRAM are enabled

    config HEAP_TRACE_HASH_MAP_USE_EXTRAM
        depends on CONFIG_SPIRAM_ALLOW_BSS_SEG_EXTERNAL_MEMORY
        depends on HEAP_TRACE_HASH_MAP
        default n

static 
#if HEAP_TRACE_HASH_MAP_USE_EXTRAM
EXT_RAM_BSS_ATTR
#endif
heap_trace_hash_list_t hash_map[(size_t)CONFIG_HEAP_TRACE_HASH_MAP_SIZE];

[chipweinberger]

Also, while we are at it, I would set the maximum hashmap size to 200k, not 10k.

After boot, I can have 20k live allocations, so I need ~30k hashmap.

[chipweinberger]

Also, IMO, the map should not be allocated until you initialize heap trace standalone, perhaps with a log statement saying how much memory is being allocated.

No need to waste memory doing nothing.

[SoucheSouche]

Hi @chipweinberger, I started working on your follow up issues (this one and #11173) today. I will let you know when they are ready and reviewed.

Maybe one remark on your comment about the hashmap size. With a hashmap array of 10K entries, if you have 20K allocations, if we consider the repartition of the calculated hashes to be homogenous across the range of array entries, it means that each list in the array would have 2 elements. I can increase the maximum number of lists in the hashmap array but in your case for example, with an array of 30k entries, you will statistically only use 66% of it given the 20k of live allocations you mentioned. In addition, each used entries will have only 1 element in the list which will probably result in lower perf. With less entries and more elements in each list, your perf will be better when using the HEAP_TRACE_LEAKS.

[chipweinberger]

With less entries and more elements in each list, your perf will be better when using the HEAP_TRACE_LEAKS.

This would be surprising to me. List traversal is slower than a single array lookup.

Still, I think 10K is too low as a maximum. Perhaps 50K at least...

[SoucheSouche]

I will increase the size to 100K and fix the external memory issue.

Also, another heap related feature will be merged soon and will allow you to store the entire heap component in flash thus saving quite a bit of internal memory but lowering the overall performances of the component.

[SoucheSouche]

@chipweinberger, the feature was reviewed and approved :) So it should be merged into master today hopefully. In the end I removed the limitation concerning the size of the hash_map.

[chipweinberger]

Great! awesome.

[chipweinberger]

Was I also able to convince you to allocate the ram at init time?

That would be ideal imo for enabling leak diagnostics at runtime (my use case).

[SoucheSouche]

@chipweinberger sorry but the memory will be allocated at compile time.