Malloc heap info

[c1728p9]

Keep track of the current and maximum space used by malloc and report
it at the end of a test.

[c1728p9]

Don't merge this yet. I pushed it up early for discussion.

[c1728p9]

/morph test

[jupe]

perhaps this functionaly should be behind some compiler flag because now this reserve 4 extra bytes every single malloc,, at least this should be documented because there is devices with very little amount of memory..

[0xc0170]

more docs in this code, why +4 (reading the code, size is stored there).

sizeof(size_t) should do better here

perhaps this functionaly should be behind some compiler flag because now this reserve 4 extra bytes every single malloc

config option?

[jupe]

cc: @kjbracey-arm @SeppoTakalo @yogpan01

[jupe]

@c1728p9: have you look up this: https://github.com/ARMmbed/nanostack-libservice/blob/master/source/nsdynmemLIB/nsdynmemLIB.c . it contains exactly same kind of functionality but little more sophisticated ;)

[c1728p9]

Hi @jupe, thanks for the link, I didn't even think to use nanostacks allocator for reference. I'll look into adding the additional statistics to this PR, since they could be useful as well.

Also, looking at the libservice, it looks like it's possible to use standard malloc rather than a custom allocator. Would it be problematic if this was turned on? This would free up a lot of ram potentially, especially for lower memory usage configurations such as 6lowpan without security.

[jupe]

I think that custom allocator was developed because standard malloc was too heavy (ticks&size) for smallest microcontrollers. @SeppoTakalo and @kjbracey-arm probably know more about this background. Anyway, I think 6lwp uses standard malloc for mbed-os builds and that why we doesn't have those statistics available for now (until this PR..) . So these statictics would be very wellcome feature for us from testing point of view.. 👍

[kjbracey]

There's no problem redirecting ns_dyn_mem_alloc to system malloc (or something else), as long as:

1. You don't mind sharing your memory pool. In some set-ups, particularly routers, network memory usage may only be limited by the size of the memory pool, so conceivably it could exhaust your main pool. Not sure how much this is really a problem in practice.

2. You know the allocator works from Nanostack background context. In mbed OS 5's case, that context is a high priority thread, so that shouldn't be an issue. In other implementations, that context may be interrupt, which is generally not supported by a system allocator.

[mbed-bot]

Result: FAILURE

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

/morph test

Output

mbed Build Number: 641

Test failed!

[c1728p9]

/morph test

[mbed-bot]

Result: FAILURE

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

/morph test

Output

mbed Build Number: 648

Test failed!

[bogdanm]

If I'm reading this PR right, it tries to get the maximum dynamic memory consumption? That'll probably work, but I'd rather take the approach in ARMmbed/ualloc#22: log each call to malloc/calloc/realloc/free and send that to a PC program that can interpret the logs. This can be used to find the maximum memory used, plus a whole lot more: a memory map, the most requested allocation sizes, checks for memory leaks, even memory allocations/module if the allocation data can be associated with a stack trace.

See also http://man7.org/linux/man-pages/man3/mallinfo.3.html (dlmalloc has an implementation of mallinfo, but I don't know if it's compiled in GCC ARM embedded's Newlib).

[jupe]

@bogdanm: I like also that approach what ualloc does but maybe with little more abstraction so that application can be decide where to dump this information and in what format. This would be also more memory friendly. Only side effect is that it require host side script which analyse that data but this is for testing purpose anyway so maybe it's accepted..

[kjbracey]

mallinfo is present and functional in GCC builds - I've used it.

[jupe]

mallinfo is present and functional in GCC builds - I've used it.

what about other builds ?

[kjbracey]

Don't think there's an equivalent for the ARM C library's allocator.

[0xc0170]

Don't think there's an equivalent for the ARM C library's allocator.

I had a look previously at this.

IAR - https://www.iar.com/support/tech-notes/general/iar-dlib-library-heap-usage-statistics/ (__iar_dlmallinfo())
ARMCC - http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.kui0099a/armlib_cihhgjad.htm (heapstats()). This heapstats() is not like dlmallinfo for IAR or GCC, not even sure its defined for ARMCC microlib 😭 this is quite sad, because if it was, we would have cross-toolchain mallinfo without much effort :-)

That'll probably work, but I'd rather take the approach in ARMmbed/ualloc#22

Good point. This PR is adding runtime stats, ualloc had just logging (we can add the logging as a separate step), or?

[c1728p9]

@kjbracey-arm, I took a look at mallinfo, and although it is present, it doesn't give some of the necessary metrics, in particular, maximum heap usage. Accoring to the man pages for mallinfo this isn't present in some environments.

usmblks

The "highwater mark" for allocated space-that is, the maximum amount of space that was ever allocated. This field is maintained only in nonthreading environments.

Furthermore, when I test it, the value usmblks always reads 0.

@bogdanm and @jupe, the approach taken in ualloc is good for certain tests, but in more complicated system could become troublesome. It uses printf to report data, but if used from multiple threads messages could interleave. Also, printf will introduce a delay, which could interfere with the tests, especially since tests are running at 9600 baud.

[bogdanm]

It uses printf to report data, but if used from multiple threads messages could interleave.

That's something we can fix though.

Also, printf will introduce a delay, which could interfere with the tests, especially since tests are running at 9600 baud.

The serial port is not the only possible way to send this data. Also, we can override the console speed of the tests, or run the memory-specific tests at a different console speed.

Again, I'm not against this PR, I just think the information it provides, while interesting, is not going to end up being very useful. When someone finds out that his program takes 100k of RAM, the next question is going to be "where is all that coming from?"

[c1728p9]

/morph test

[mbed-bot]

Result: FAILURE

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

/morph test

Output

mbed Build Number: 652

Test failed!

[bogdanm]

I think you might be missing one case: according to realloc man page, if ptr is not NULL and size is 0, the call to realloc is equivalent to free(ptr).

[bogdanm]

Also, when ptr is NULL, realloc is equivalent with malloc(size). I'm not sure you implement this either.

[c1728p9]

"according to realloc man page, if ptr is not NULL and size is 0, the call to realloc is equivalent to free(ptr)." - This is implementation defined, and not required to be null. From the man pages here:
"If size was equal to 0, either NULL or a pointer suitable to be passed to free() is returned."

"Also, when ptr is NULL, realloc is equivalent with malloc(size). I'm not sure you implement this either." - This is both implemented and tested with a test case.

[bogdanm]

"If size was equal to 0, either NULL or a pointer suitable to be passed to free() is returned."

You're talking about the return value, I'm talking about this part: if size is equal to zero, and ptr is not NULL, then the call is equivalent to free(ptr).

This is both implemented and tested with a test case.

According to your code, when ptr is NULL, you skip over the first if (line 548), allocate a new buffer (line 556), and then proceed to copy data into this new buffer from the NULL pointer (line 563), then you execute free(NULL) on line 564. It might appear that it works, but I don't think that's the intended behaviour?

[c1728p9]

Ah I see what you mean. This behavior is still implementation defined since the standard allows a non-null pointer to be returned:
http://en.cppreference.com/w/c/memory/realloc
If new_size is zero, the behavior is implementation defined (null pointer may be returned (in which case the old memory block is not freed), or some non-null pointer may be returned that may not be used to access storage).

http://www.cplusplus.com/reference/cstdlib/realloc/
For C99/C++11 If size is zero, the return value depends on the particular library implementation: it may either be a null pointer or some other location that shall not be dereferenced.

I would prefer to be on the stricter side here, so if there could be a memory leak on some implementations, that shows up in the testing.

The memcpy should have a size of zero (so nothing is copied) and free is called on a null pointer, which is defined behavior which does nothing. If you think it would make it clearer, I could do an explicit check for null here.

[bogdanm]

Whichever you'd like. The free(NULL) call will look strange if the memory tracer is also enabled; also, I believe that when something is listed as "implementation defined", man realloc really helps you to figure out which implementation to choose. But in the end one cannot argue with "implementation defined" 😄

[jupe]

When someone finds out that his program takes 100k of RAM, the next question is going to be "where is all that coming from?"

I agree. What about solution which are kind of mix of approach from this PR, ualloc and nsdynmemlib. --> If Application set stats callback pointer for mbed-os then os start calling that function every time when calling memory allocation functions (free/malloc/realloc/..). Then it's application responsibility to do what ever it want to do with it...e.g. it could print directly to serial port, or collect just summary. ... This kind of approach would be also thread safe from mbed-os point of view and by default it doesn't take more memory either ticks... Actually it allows also to create mbed-client resource which represent device RAM usage for cloud ;).. How that sounds like ? This is just a wild idea .. :)

[sg-]

#2475

[bogdanm]

@bogdanm, what are your thoughts.

Agreed. Both need to exist. #2487 adds traces after the memory operation executes, while this PR needs to modify the way the memory operation works, in order to record block size information. So they need to exist together and we need to be able to turn them on or off individually.

[adbridge]

I do wonder just how much use these stats are in reality? They're only going to be taken at the end of a test (i.e. end of the binary execution) and nothing more will run after that point so all we really see is how much malloc and heap space is left at the end of execution.

[jupe]

@adbridge: it depend on what framework you are using.. e.g. it is possible to "poll" memory consumption while testing, but might not be so useful/reality with greentea..

[bogdanm]

LGTM 👍

[c1728p9]

Typo

[SeppoTakalo]

I don't see the most usefull information here, which is the maximum size of the heap in use.
If you just measure allocated space, you ignore all the fragmentation and other stuff that happens inside the allocator.
You might want to keep track of the "high watermark" of the heap. That would be the actual memory usage of application.

[kjbracey]

"Largest contiguous block free" is properly the stat you want to catch fragmentation.

[c1728p9]

Hi @SeppoTakalo, the worst case heap is recorded and can be seen in the max_size field of the mbed_stats_heap_t structure.

@kjbracey-arm, unfortunately this metric is tricky to get. With @bogdanm's memory logging and some host processing it should be possible to determine this.

[SeppoTakalo]

Hi @c1728p9, the max_size seems to be the maximum size of objects allocated from malloc. Which is not the same thing as how much memory malloc uses.
What I mean about the "high water mark" is the topmost pointer in the heap used.
If you say max_alloc is 1kB, it does not mean that 1kB heap size is enough. We need to have value that includes also the fragmentation of the heap and internal structures used by malloc.

Basically, if you allocate one object size of 1000B, this says max_alloc is 1kB. But if you alloc 1000 objects, each of size 1, this will still tell you that max_alloc is 1kB. But in reality, between every object there is data structure used by malloc and padding of course. Therefore the max_alloc is actually misleading value. It gives too optimistic results of memory usage and does not count in the fragmentation of the heap.

We need something that can be used for developers to REALLY know how big the heap has been. Not just sum of all the sizes of allocated objects, but the total cost of allocation, including internal structures, padding and fragmentation.
That information can then be used to tune the linker files, for example, or decide which chip to use.

[c1728p9]

Hi @SeppoTakalo, I see what you mean. This can vary by the heap implementation and order of allocation, which is not always consistent. The current mechanism provides an exact count of the bytes allocated by the application. This makes it easier to tell when a change increases or decreases heap usage. Also, these statistics are straight forward to get and match those which nanostack supports - https://github.com/ARMmbed/nanostack-libservice/blob/master/source/nsdynmemLIB/nsdynmemLIB.c.

If more precise info is needed, @bogdanm's implementation can be used with the MBED_MEM_TRACING_ENABLED flag.

Finally, if you know a straight forward way to get the high water mark for GCC and ARMCC feel free to leave a comment here (or open a PR 😄). This would definitely add value to the stats. I don't know of a straight forward way to get this info so it isn't in this PR.