Skip to content
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.

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

Add malloc_count Component to track heap usage #1743

Merged
merged 5 commits into from
Jul 8, 2019

Conversation

mikee47
Copy link
Contributor

@mikee47 mikee47 commented Jul 4, 2019

Currently, system_get_free_heap_size() doesn't return anything meaningful. This PR fixes that using a Component based on malloc_count https://panthema.net/2013/malloc_count/.

I've also added a couple of functions to enable logging. To use it, add #include <malloc_count.h> and call MallocCount::enableLogging() and MallocCount::setLogThreshold() as required.

The Component also works for the Esp8266 (standard or custom heaps). Enable by adding COMPONENT_DEPENDS += malloc_count to the project's component.mk file.

The Component is enabled by default for the Host emulator, but can be disabled using ENABLE_MALLOC_COUNT=0.

@slaff slaff added this to the 3.9.0 milestone Jul 4, 2019
@mikee47
Copy link
Contributor Author

mikee47 commented Jul 5, 2019

It would probably be useful to add an option to disable this if using valgrind, ENABLE_MALLOC_COUNT,
but I think should be enabled by default.

As the API is only available for Host builds it needs to be conditionally compiled (using #ifdef ARCH_HOST). An alternative is to make this a generic Component so it can be used on any architecture, with a common API. However, the ESP32 (and Esp8266 RTOS) use Espressif's 'capability heap' and includes tracing support. As a separate issue then, perhaps we should take a look at that and see if it offers any improvements over the existing heap allocation options?

@slaff
Copy link
Contributor

slaff commented Jul 5, 2019

It would probably be useful to add an option to disable this if using valgrind, ENABLE_MALLOC_COUNT,

Yes, please.

@slaff
Copy link
Contributor

slaff commented Jul 5, 2019

The Windows build is failing. That is probably due to the concurrency and the way windows handles files. I saw "Error 5" in the error output which lead me to this post: https://stackoverflow.com/questions/14201870/cmake-fails-to-compile-simple-test-error-5-access-denied.

@mikee47
Copy link
Contributor Author

mikee47 commented Jul 5, 2019

I've logged in via RDP to find out what's going on here. There's some mingw startup code which calls free on memory which wasn't allocated via malloc_count, which leads to a sentinel check failure. The code then goes on to call __real_free() on an invalid location (ptr - 16) hence access denied. I'll do some more testing and fix this.

@slaff slaff removed this from the 3.9.0 milestone Jul 5, 2019
@mikee47 mikee47 force-pushed the feature/malloc_count branch from d08230b to dedfeb9 Compare July 6, 2019 11:39
@mikee47 mikee47 changed the title Add simple heap checking for Host emulator Add simple heap checking Jul 6, 2019
@mikee47 mikee47 force-pushed the feature/malloc_count branch from dedfeb9 to c97ee62 Compare July 6, 2019 11:54
@mikee47 mikee47 changed the title Add simple heap checking Add malloc_count Component to track heap usage Jul 6, 2019
@mikee47 mikee47 force-pushed the feature/malloc_count branch from c97ee62 to 161114e Compare July 6, 2019 19:42
@mikee47 mikee47 force-pushed the feature/malloc_count branch from 161114e to f5333e5 Compare July 7, 2019 08:21
@slaff slaff added this to the 3.9.0 milestone Jul 7, 2019
@slaff
Copy link
Contributor

slaff commented Jul 7, 2019

@mikee47 Is this PR ready for merging?

@mikee47
Copy link
Contributor Author

mikee47 commented Jul 7, 2019

@slaff Yep, all done thanks.

@slaff slaff removed the 3 - Review label Jul 8, 2019
@slaff slaff merged commit 6b0c71b into SmingHub:develop Jul 8, 2019
@mikee47 mikee47 deleted the feature/malloc_count branch July 14, 2019 20:05
@slaff slaff mentioned this pull request Sep 28, 2019
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants