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 heap stats for IAR #4965

Merged
merged 1 commit into from Sep 22, 2017

Conversation

Projects
None yet
8 participants
@deepikabhavnani
Contributor

deepikabhavnani commented Aug 23, 2017

Heap stats were not calculated for IAR, since we didn't have mechanism to hook IAR functions.

IAR has internal heap statistic data, which can be enabled and used. Below is the reference link
https://www.iar.com/support/tech-notes/general/iar-dlib-library-heap-usage-statistics/

Only current heap size (with 8 bytes of overhead per malloc) and max heap size is reported.
Below is the output for simple malloc program allocating and freeing 1000 bytes of memory

Allocating 1000 bytes
Current heap: 1008
Total heap size: 65536
Freeing 1000 bytes
Current heap after: 0
Total heap size: 65536

@deepikabhavnani deepikabhavnani force-pushed the deepikabhavnani:IAR_heap_stats branch Aug 25, 2017

@deepikabhavnani deepikabhavnani requested a review from c1728p9 Aug 28, 2017

@YarivCol

This comment has been minimized.

Contributor

YarivCol commented Aug 28, 2017

It will be nicer if we change dmalloc.c, add wrap function to malloc, free, etc...
In that case we will have full support to heap trace and heap stats.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Aug 31, 2017

It will be nicer if we change dmalloc.c, add wrap function to malloc, free, etc...
In that case we will have full support to heap trace and heap stats.

I believe this is not just for IAR, is it? To wrap all functions to enable trace/stats.
We used to have in dlmalloc with our own allocators ualloc (https://github.com/ARMmbed/ualloc)

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Sep 4, 2017

Just in case, as the referenced page above is from 2015, this has no depedency on IAR 8? I would say no it does not.

@c1728p9 Please review

@deepikabhavnani

This comment has been minimized.

Contributor

deepikabhavnani commented Sep 6, 2017

This is just for IAR and not dependent on IAR 8.x version.

@deepikabhavnani

This comment has been minimized.

Contributor

deepikabhavnani commented Sep 14, 2017

@YarivCol - Yes it would be good to update dlmalloc.c and provide full support.

@sg- @bulislaw @0xc0170 @c1728p9 - Do we have any concerns in adding IAR code base file into mbed-os? dlmalloc.c code will be for only IAR support and we will have to re-visit this file everytime we upgrade to new IAR version, to make sure nothing breaks.

I compared dlmalloc.c in IAR7.8 and IAR 8.11 versions, and only change was an additional API added
dlaligned_alloc which is exact duplicate of existing aligment API dlmemalign. I don;t see any issues in porting for both IAR versions.

@sg-

This comment has been minimized.

Member

sg- commented Sep 14, 2017

I dont think this is a file we should check in, rather ask IAR for a hook.

@TTornblom

This comment has been minimized.

Contributor

TTornblom commented Sep 15, 2017

Isn't this something where one could use "$Super$$ and $Sub$$" (http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0377g/pge1362065967698.html) to hook into malloc()/free() etc?

Example:

#include <stdlib.h>
#include <stdio.h>

int main()
{
  int * p = (int *)malloc(100);
  int * p2 = (int *)malloc(100);
  p[2] = 45;
  free(p2);
  return p[3];
} 

extern void * $Super$$__iar_dlmalloc(size_t);

void * $Sub$$__iar_dlmalloc(size_t w)
{
  printf("Called with %d\n", w);
  return $Super$$__iar_dlmalloc(w);
}
@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Sep 15, 2017

@TTornblom Github uses ``` instead of --- for code comments, and you can get syntax highlighting if you specify the language after the first ```. For example ```C.

@TTornblom

This comment has been minimized.

Contributor

TTornblom commented Sep 15, 2017

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Sep 15, 2017

Oh, FYI @TTornblom, your suggestion will work for Arm Compiler {5|6}, and this PR is targeting IAR EWARM. I think they may have a similar wrapping feature, but your code snippet is not applicable to this PR. My mistake.

@TTornblom

This comment has been minimized.

Contributor

TTornblom commented Sep 15, 2017

I work for IAR and this example was provided by IAR engineering, so works with IAR EWARM :)

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Sep 15, 2017

@TTornblom Ah! My bad. Glad to have you in this discussion.

@deepikabhavnani deepikabhavnani force-pushed the deepikabhavnani:IAR_heap_stats branch Sep 15, 2017

@deepikabhavnani

This comment has been minimized.

Contributor

deepikabhavnani commented Sep 15, 2017

@sg- @c1728p9 - Please review new implementation for heap stats as per suggestion from @TTornblom

platform/mbed_alloc_wrappers.cpp Outdated
@@ -329,6 +329,122 @@ extern "C" void $Sub$$free(void *ptr) {
#endif // #if defined(MBED_MEM_TRACING_ENABLED) || defined(MBED_HEAP_STATS_ENABLED)
/******************************************************************************/
/* IAR memory allocation wrappers */
/******************************************************************************/
#elif defined(__ICCARM__) // #if defined(TOOLCHAIN_GCC)

This comment has been minimized.

@sg-

sg- Sep 15, 2017

Member

do we need this lingering comment?

This comment has been minimized.

@deepikabhavnani

deepikabhavnani Sep 15, 2017

Contributor

@sg- Nah, just kept to have consistency.. Will remove

platform/mbed_alloc_wrappers.cpp Outdated
/******************************************************************************/
#elif defined(__ICCARM__)

This comment has been minimized.

@c1728p9

c1728p9 Sep 16, 2017

Contributor

You might be able to combine the ARM and IAR code and just define the function names. Something like this:

#elif defined(TOOLCHAIN_ARM)
#define SUPER_MALLOC    $Super$$malloc
#define SUB_MALLOC      $Sub$$malloc
...

#elif defined(__ICCARM__)
#define SUPER_MALLOC    $Super$$__iar_dlmalloc
#define SUB_MALLOC      $Sub$$__iar_dlmalloc
...

#endif

extern "C" {
    void *SUPER_MALLOC(size_t);
    void *SUPER_REALLOC(void *ptr, size_t size);
    void *SUPER_CALLOC(size_t nmemb, size_t size);
    void SUPER_FREE(void *ptr);
}

extern "C" void *SUB_MALLOC(size_t size) {
    void *ptr = NULL;
...
}
...

This comment has been minimized.

@deepikabhavnani

deepikabhavnani Sep 16, 2017

Contributor

I intentionally separated IAR as was not sure if something additional would be required or not. Since its now working can merge ARM and IAR implementation

@deepikabhavnani deepikabhavnani force-pushed the deepikabhavnani:IAR_heap_stats branch to 459e7d4 Sep 18, 2017

@deepikabhavnani

This comment has been minimized.

Contributor

deepikabhavnani commented Sep 18, 2017

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Sep 18, 2017

Result: ABORTED

Your command has finished executing! Here's what you wrote!

/morph test

@deepikabhavnani

This comment has been minimized.

Contributor

deepikabhavnani commented Sep 21, 2017

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Sep 21, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1352

All builds and test passed!

@0xc0170 0xc0170 merged commit b72bd08 into ARMmbed:master Sep 22, 2017

4 checks passed

Cam-CI uvisor Build & Test Success
Details
ci/morph-test Job has completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@deepikabhavnani deepikabhavnani deleted the deepikabhavnani:IAR_heap_stats branch Dec 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment