Introduce configTRACK_TASK_MEMORY_ALLOCATIONS to track dynamic allocations per task #353

RichardBarry · 2021-06-14T01:54:51Z

Description

Introduce configTRACK_TASK_MEMORY_ALLOCATIONS. When this is set to 1 and either heap_4 or heap_5 is used then the number of dynamic memory allocations, the number of dynamic memory frees, the total amount of heap memory allocated by a task but not yet freed (by any task), and the maximum amount of heap memory a task has ever had allocated at one time are all tracked on a task by task basis. This additional information is returned in the TaskStatus_t structure populated by the vTaskGetInfo() and the uxTaskGetSystemState() API functions.

uxBasePriority is now only included in the TaskStatus_t structure if configUSE_MUTEXES is set to 1. ulRunTimeCounter is now only included in the TaskStatus_t structure if configGENERATE_RUN_TIME_STATS is set to 1.

Update how the TCB members get initialised. Previously each member was initialised separately. Now, as there are new members, memset() the entire structure to 0 when the structure is allocated, then only explicitly initialise structure members that are not initialised to zero.

Test Steps

Add prvExerciseTaskHeapAllocationStats() to the MSVC Win32 and MPS2 QEMU IAR demos - upstreamed to the FreeRTOS/FreeRTOS repo in a separate PR.

Related Issue

The PR that adds prvExerciseTaskHeapAllocationStats() to the FreeRTOS/FreeRTOS repo.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

…and either heap_4 or heap_5 is used then the number of dynamic memory allocations, the number of dynamic memory frees, the total amount of heap memory allocated by a task but not yet freed (by any task), and the maximum amount of heap memory a task has ever had allocated at one time are all tracked on a task by task basis. This additional information is returned in the TaskStatus_t structure populated by the vTaskGetInfo() and the uxTaskGetSystemState() API functions. uxBasePriority is now only included in the TaskStatus_t structure if configUSE_MUTEXES is set to 1. ulRunTimeCounter is now only included in the TaskStatus_t structure if configGENERATE_RUN_TIME_STATS is set to 1. Update how the TCB members get initialised. Previously each member was initialised separately. Now, as there are new members, memset() the entire structure to 0 when the structure is allocated, then only explicitly initialise structure members that are not initialised to zero.

codecov · 2021-06-14T01:58:36Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.15%. Comparing base (8e2dd5b) to head (2d6724e).
Report is 502 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #353   +/-   ##
=======================================
  Coverage   92.15%   92.15%           
=======================================
  Files           4        4           
  Lines        1274     1274           
  Branches      343      343           
=======================================
  Hits         1174     1174           
  Misses         53       53           
  Partials       47       47

Flag	Coverage Δ
unittests	`92.15% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…EMU IAR demos to exercise the function that track memory allocations on a task by task basis as introduced by FreeRTOS/FreeRTOS-Kernel#353.

RichardBarry · 2021-06-14T02:15:57Z

Spell check is failing because of variable names. Is spell check meant to check the C code, or just the comments?

mingyue86010 · 2021-06-14T18:59:41Z

Hi @RichardBarry I just checked with team. It looks like the current spell checker checks any line of code with comment. So if a C code line contains a comment it will check the spell of entire line inclduing the code itself. We are look for some more generic spell checker to replace the current one and hopfully can resolve the problem.

But I'm going to unblock this PR by adding exclusion of those words for now.

mingyue86010 · 2021-06-15T18:58:27Z

/bot run checks

RichardBarry · 2021-12-30T17:47:55Z

tasks.c

+
+    TaskHandle_t xTaskCreateStatic( TaskFunction_t pxTaskCode,
+                                    const char * const pcName, /*lint !e971 Unqualified char types are allowed for strings and single characters only. */
+                                    const uint32_t ulSt


Looks like ulTaskGetIdleRunTimePercent() is missing here.

RichardBarry · 2021-12-30T17:49:19Z

include/task.h

+    #endif
+
+    #if ( configGENERATE_RUN_TIME_STATS == 1 )
+        uint32_t ulRunTimeCounter; /* The total run time allocated to the task so far, as defined by the run time stats clock.  See https://www.FreeRTOS.org/rtos-run-time-stats.html.  Only valid when configGENERATE_RUN_TIME_STATS is defined as 1 in FreeRTOSConfig.h. */


ulRunTimeCounter should now have type configRUN_TIME_COUNTER_TYPE so users can use a 64-bit counter if they wish.

RichardBarry · 2021-12-30T17:51:14Z

tasks.c

+
+    TaskHandle_t xTaskCreateStatic( TaskFunction_t pxTaskCode,
+                                    const char * const pcName, /*lint !e971 Unqualified char types are allowed for strings and single characters only. */
+                                    const uint32_t ulSt


[edit] This comment should be in xTaskUpdateHeapAllocationStats() where the total heap held gets incremented. I don't think there is a risk to the value overflowing (which the comment says should be impossible under normal control flows anyway), but it might be beneficial to cap to 0xffffffff if that does occur. [/edit]

RichardBarry · 2021-12-30T17:53:14Z

tasks.c

+
+    TaskHandle_t xTaskCreateStatic( TaskFunction_t pxTaskCode,
+                                    const char * const pcName, /*lint !e971 Unqualified char types are allowed for strings and single characters only. */
+                                    const uint32_t ulSt


Preference is to use "!= pdFALSE", rather than "== pdTRUE" as it can generate more efficient code, especially on devices with a register that is always zero.

RichardBarry · 2021-12-30T17:55:21Z

tasks.c

+
+    TaskHandle_t xTaskCreateStatic( TaskFunction_t pxTaskCode,
+                                    const char * const pcName, /*lint !e971 Unqualified char types are allowed for strings and single characters only. */
+                                    const uint32_t ulSt


Don't think there is a risk from underflow of this variable, but perhaps set xHeapBytesCurrentlyAllocated 0 if that occurs.

RichardBarry · 2021-12-30T17:56:31Z

tasks.c

+
+    TaskHandle_t xTaskCreateStatic( TaskFunction_t pxTaskCode,
+                                    const char * const pcName, /*lint !e971 Unqualified char types are allowed for strings and single characters only. */
+                                    const uint32_t ulSt


This is inconsistent and brings in a dependency on limits.h. The previous function defined const uint32_t ulMaxUint32 = 0xffffffffUL; which is preferable to using UINT32_MAX.

Skptak · 2023-09-20T19:51:20Z

I think this work has been done at this point?
It looks like we have traceMALLOC and traceFREE calls inside of the heap files now
Should this PR be closed?

sonarqubecloud · 2024-01-05T17:32:10Z

Quality Gate failed

Failed conditions

D Reliability Rating on New Code (required ≥ A)
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

…reeRTOS#353) To avoid silent errors, configNETWORK_INTERFACE_TO_USE should be defaulted to 0L.

aggarg · 2024-09-17T06:25:59Z

Closing this PR as it is possible to track allocations using trace macros -

#define NUM_ALLOCATION_ENTRIES          1024
#define NUM_PER_TASK_ALLOCATION_ENTRIES 32

/*
 * +-----------------+--------------+----------------+-------------------+
 * | Allocating Task | Entry in use | Allocated Size | Allocated Pointer |
 * +-----------------+--------------+----------------+-------------------+
 * |                 |              |                |                   |
 * +-----------------+--------------+----------------+-------------------+
 * |                 |              |                |                   |
 * +-----------------+--------------+----------------+-------------------+
 */
typedef struct AllocationEntry
{
    BaseType_t inUse;
    TaskHandle_t allocatingTask;
    size_t allocatedSize;
    void * allocatedPtr;
} AllocationEntry_t;

AllocationEntry_t allocationEntries[ NUM_ALLOCATION_ENTRIES ];

/*
 * +------+-----------------------+----------------------+
 * | Task | Memory Currently Held | Max Memory Ever Held |
 * +------+-----------------------+----------------------+
 * |      |                       |                      |
 * +------+-----------------------+----------------------+
 * |      |                       |                      |
 * +------+-----------------------+----------------------+
 */
typedef struct PerTaskAllocationEntry
{
    TaskHandle_t task;
    size_t memoryCurrentlyHeld;
    size_t maxMemoryEverHeld;
} PerTaskAllocationEntry_t;

PerTaskAllocationEntry_t perTaskAllocationEntries[ NUM_PER_TASK_ALLOCATION_ENTRIES ];

/*-----------------------------------------------------------*/

void TrackAllocation( size_t allocatedSize, void * ptr );
void TraceFree( void * ptr );

/*-----------------------------------------------------------*/

void TrackAllocation( size_t allocatedSize, void * ptr )
{
    size_t i;
    TaskHandle_t allocatingTask;
    AllocationEntry_t * pAllocationEntry = NULL;
    PerTaskAllocationEntry_t * pTaskAllocationEntry = NULL;

    if( xTaskGetSchedulerState() != taskSCHEDULER_NOT_STARTED )
    {
        allocatingTask = xTaskGetCurrentTaskHandle();

        for( i = 0; i < NUM_ALLOCATION_ENTRIES; i++ )
        {
            if( allocationEntries[ i ].inUse == pdFALSE )
            {
                pAllocationEntry = &( allocationEntries[ i ] );
                break;
            }
        }

        /* Do we already have an entry in the per task table? */
        for( i = 0; i < NUM_PER_TASK_ALLOCATION_ENTRIES; i++ )
        {
            if( perTaskAllocationEntries[ i ].task == allocatingTask )
            {
                pTaskAllocationEntry = &( perTaskAllocationEntries[ i ] );
                break;
            }
        }

        /* We do not have an entry in the per task table. Find an empty slot. */
        if( pTaskAllocationEntry == NULL )
        {
            for( i = 0; i < NUM_PER_TASK_ALLOCATION_ENTRIES; i++ )
            {
                if( perTaskAllocationEntries[ i ].task == NULL )
                {
                    pTaskAllocationEntry = &( perTaskAllocationEntries[ i ] );
                    break;
                }
            }
        }

        /* Ensure that we have space in both the tables. */
        configASSERT( pAllocationEntry != NULL );
        configASSERT( pTaskAllocationEntry != NULL );

        pAllocationEntry->allocatingTask = allocatingTask;
        pAllocationEntry->inUse = pdTRUE;
        pAllocationEntry->allocatedSize = allocatedSize;
        pAllocationEntry->allocatedPtr = ptr;

        pTaskAllocationEntry->task = allocatingTask;
        pTaskAllocationEntry->memoryCurrentlyHeld += allocatedSize;
        if( pTaskAllocationEntry->maxMemoryEverHeld < pTaskAllocationEntry->memoryCurrentlyHeld )
        {
            pTaskAllocationEntry->maxMemoryEverHeld = pTaskAllocationEntry->memoryCurrentlyHeld;
        }
    }
}

/*-----------------------------------------------------------*/

void TraceFree( void * ptr )
{
    size_t i;
    AllocationEntry_t * pAllocationEntry = NULL;
    PerTaskAllocationEntry_t * pTaskAllocationEntry = NULL;

    for( i = 0; i < NUM_ALLOCATION_ENTRIES; i++ )
    {
        if( ( allocationEntries[ i ].inUse == pdTRUE ) &&
            ( allocationEntries[ i ].allocatedPtr == ptr ) )
        {
            pAllocationEntry = &( allocationEntries [ i ] );
            break;
        }
    }

    /* Attempt to free a block that was never allocated. */
    configASSERT( pAllocationEntry != NULL );

    for( i = 0; i < NUM_PER_TASK_ALLOCATION_ENTRIES; i++ )
    {
        if( perTaskAllocationEntries[ i ].task == pAllocationEntry->allocatingTask )
        {
            pTaskAllocationEntry = &( perTaskAllocationEntries[ i ] );
            break;
        }
    }

    /* An entry must exist in the per task table. */
    configASSERT( pTaskAllocationEntry != NULL );

    pTaskAllocationEntry->memoryCurrentlyHeld -= pAllocationEntry->allocatedSize;

    pAllocationEntry->inUse = pdFALSE;
    pAllocationEntry->allocatingTask = NULL;
    pAllocationEntry->allocatedSize = 0;
    pAllocationEntry->allocatedPtr = NULL;
}

/*-----------------------------------------------------------*/

The following goes in FreeRTOSConfig.h:

void TrackAllocation( size_t allocatedSize, void * ptr );
void TraceFree( void * ptr );

#define traceMALLOC( pvReturn, xAllocatedBlockSize ) \
TrackAllocation( xAllocatedBlockSize, pvReturn )

#define traceFREE( pv, xAllocatedBlockSize ) \
TraceFree( pv )

Panometric · 2024-10-02T16:05:08Z

@aggarg Will these macros be defined in a future release? Seems odd to close a PR with a workaround only documented in a closed PR. I think there is a strong case for having this functionality in the released version.

aggarg · 2024-10-03T16:23:20Z

@Panometric, the purpose of the above example code is to show the malloc tracking can be implemented in the application using trace macros. We will consider providing a sample with these definitions. I am interested to know your use case. Would you please be kind enough to share what you want to track and for what purpose?

Panometric · 2024-10-03T17:01:26Z

@aggarg Thanks, I do believe there is a general need since finding the uses of the heap can be quite daunting. In our case we are kind of forced to share the heap with a binary library for BLE that only provides hooks for malloc/free that are routed back through FreeRTOS. Allocated buffers are passed in that should be released by the library once sent. It only leaks under certain conditions, like when comms were in progress when the connection drops. So it's difficult problem to recreate. Therefore, we would use this do a post mortem, proving the the library vendor that they are causing the leak.

aggarg · 2024-10-09T07:37:33Z

Thank you for sharing your use case. Just one more question - do you keep this enabled in your production application or do you use it during development only?

Panometric · 2024-10-09T22:08:16Z

Hard to say for sure, probably only QA versions, since it introduces some load and memory usage. But we would likely preserve the tracking data in for reporting after reset caused a failed malloc.

aggarg · 2024-10-11T09:53:06Z

Thank you for sharing.

RichardBarry requested a review from a team as a code owner June 14, 2021 01:54

RichardBarry mentioned this pull request Jun 14, 2021

Add prvExerciseTaskHeapAllocationStats() to the MSVN Win32 and MPS2 Q… FreeRTOS/FreeRTOS#624

Closed

mingyue86010 mentioned this pull request Jun 14, 2021

Update spell checker exclusion. #354

Merged

RichardBarry and others added 4 commits June 15, 2021 15:31

Update lexicon.txt

d122094

Merge branch 'main' into track-memory-allocation-counts-in-tcb

0829685

uncrustified

b04bb01

uncrustified with older version

abd0383

n9wxu previously approved these changes Dec 31, 2021

View reviewed changes

Merge branch 'main' into track-memory-allocation-counts-in-tcb

2d6724e

RichardBarry commented Jan 4, 2022

View reviewed changes

github-actions bot dismissed n9wxu’s stale review via 42257f9 September 20, 2023 19:52

Skptak force-pushed the track-memory-allocation-counts-in-tcb branch from 42257f9 to 2d6724e Compare January 5, 2024 17:31

laroche pushed a commit to laroche/FreeRTOS-Kernel that referenced this pull request Apr 18, 2024

Update configNETWORK_INTERFACE_TO_USE default value for demos to 0L (F…

553d448

…reeRTOS#353) To avoid silent errors, configNETWORK_INTERFACE_TO_USE should be defaulted to 0L.

n9wxu mentioned this pull request May 17, 2024

[BUG] Violation of MISRA2012 rule 17.11 - A function that never returns should be declared with a _Noreturn function specifier #1057

Closed

aggarg closed this Sep 17, 2024

zylalx1 mentioned this pull request Nov 7, 2024

Outdated Information in Section 3.3.5 - Collecting Per-task Heap Usage Statistics FreeRTOS/FreeRTOS-Kernel-Book#100

Closed

Introduce configTRACK_TASK_MEMORY_ALLOCATIONS to track dynamic allocations per task #353

Introduce configTRACK_TASK_MEMORY_ALLOCATIONS to track dynamic allocations per task #353

Uh oh!

Conversation

RichardBarry commented Jun 14, 2021

Description

Test Steps

Related Issue

Uh oh!

codecov bot commented Jun 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

RichardBarry commented Jun 14, 2021

Uh oh!

mingyue86010 commented Jun 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mingyue86010 commented Jun 15, 2021

Uh oh!

RichardBarry Dec 30, 2021

Choose a reason for hiding this comment

Uh oh!

RichardBarry Dec 30, 2021

Choose a reason for hiding this comment

Uh oh!

RichardBarry Dec 30, 2021

Choose a reason for hiding this comment

Uh oh!

RichardBarry Dec 30, 2021

Choose a reason for hiding this comment

Uh oh!

RichardBarry Dec 30, 2021

Choose a reason for hiding this comment

Uh oh!

RichardBarry Dec 30, 2021

Choose a reason for hiding this comment

Uh oh!

Skptak commented Sep 20, 2023

Uh oh!

sonarqubecloud bot commented Jan 5, 2024

Quality Gate failed

Uh oh!

aggarg commented Sep 17, 2024

Uh oh!

Panometric commented Oct 2, 2024

Uh oh!

aggarg commented Oct 3, 2024

Uh oh!

Panometric commented Oct 3, 2024

Uh oh!

aggarg commented Oct 9, 2024

Uh oh!

Panometric commented Oct 9, 2024

Uh oh!

aggarg commented Oct 11, 2024

Uh oh!

Uh oh!

codecov bot commented Jun 14, 2021 •

edited

Loading

mingyue86010 commented Jun 14, 2021 •

edited

Loading