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

Feature/heap fragmentation #1452

Merged
merged 8 commits into from Jan 22, 2018

Conversation

@m-mcgowan
Copy link
Contributor

commented Dec 21, 2017

Problem

The application does not know when the system has ran out of memory, and cannot determine if it is due to total exhaustion of the heap or due to a fragmented heap.

Solution

Adds a new system event, out_of_memory, which is fired when a heap request fails. The flags parameter for the posted event is the size of the block requested. The event is posted on the application thread - recursive invocation of this event (in the case where attempting to post the event itself causes an out of memory error) raises a PANIC.

The HAL_Core_Runtime_Info function has the struct extended with an additional field largest_free_block_heap which is the size of the largest free block in the heap (i.e. the largest block that can be allocated.) With a fragmented heap, the largest block may be significantly less than the amount of free memory.

The integration tests verify the event is fired, and verify how the heap behaves when it is fragmented.

Steps to Test

  • wiring/no_fixture SYSTEM_06*, SYSTEM_07*, built with USE_THREADING=y && n

Example App

todo - do not merge until the docs PR is linked here.

Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

@m-mcgowan m-mcgowan added this to the 0.8.0-rc.2 milestone Dec 21, 2017

@m-mcgowan m-mcgowan requested a review from avtolstoy Dec 21, 2017

@m-mcgowan m-mcgowan requested a review from sergeuz Jan 17, 2018

@m-mcgowan m-mcgowan removed the do not merge label Jan 17, 2018

@m-mcgowan m-mcgowan force-pushed the feature/heap_fragmentation branch from 82d5f95 to 18700a8 Jan 18, 2018

extern void vApplicationMallocFailedHook( void );
vApplicationMallocFailedHook();
extern void vApplicationMallocFailedHook( size_t );
vApplicationMallocFailedHook(xWantedSize);

This comment has been minimized.

Copy link
@sergeuz

sergeuz Jan 18, 2018

Member

Shouldn't we report the originally requested size here (xWantedSize - xHeapStructSize) similarly to the Photon implementation?

Another problem here is that FreeRTOS invokes vApplicationMallocFailedHook() when the size passed to pvPortMalloc() is 0. Given that our current malloc() is a trivial wrapper over pvPortMalloc() this would cause the out_of_memory event to be generated for the void* ptr = malloc(0) statement, which is perfectly valid in terms of the standard library.

This comment has been minimized.

Copy link
@m-mcgowan

m-mcgowan Jan 18, 2018

Author Contributor

yes, in fact, I wanted to move that file to a common location and avoid the duplication. I will do that. And I will add a 0 size check so that the system event is not fired. Thanks for picking that up - really good catch!

m-mcgowan and others added 6 commits Dec 13, 2017
queue up the next built bootloader version (so we don't forget to bum…
…p it) but only update the dependency when the bootloader is built.
fix for hard fault on electron calling Wire1.begin() on the electron,…
… due to the fact that Wire and Wire1 share the same peripheral, configured for different pins.
adds a system event for heap allocation failure that reports the size…
… of the block requsted, adds the largest heap block to the runtime info, and adds integration tests for these.

@m-mcgowan m-mcgowan force-pushed the feature/heap_fragmentation branch from 18700a8 to 09a3081 Jan 22, 2018

remove duplicate heap_4_lock file, and do not invoke the malloc faile…
…d handler when the allocation size is 0

@m-mcgowan m-mcgowan force-pushed the feature/heap_fragmentation branch from 09a3081 to b2952cd Jan 22, 2018


#if( configUSE_MALLOC_FAILED_HOOK == 1 )
{
if( pvReturn == NULL && (xWantedSize-xHeapStructSize)>0)

This comment has been minimized.

Copy link
@sergeuz

sergeuz Jan 22, 2018

Member

(xWantedSize-xHeapStructSize)>0 is always true given that both operands are unsigned integers, and xWantedSize is adjusted only if it's larger than 0

This comment has been minimized.

Copy link
@m-mcgowan

m-mcgowan Jan 22, 2018

Author Contributor

strange - the tests show that the behaviour is as expected.

This comment has been minimized.

Copy link
@sergeuz

sergeuz Jan 22, 2018

Member

hm, SYSTEM_08* fails for me though

@m-mcgowan m-mcgowan merged commit 4fb451b into develop Jan 22, 2018

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@m-mcgowan m-mcgowan deleted the feature/heap_fragmentation branch Jan 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.