[Feature Request] Pass core ID to SMP port lock functions

[felixvanoost]

Is your feature request related to a problem? Please describe.
In SMP mode, several functions must execute in critical sections by making sequential calls to portGET_TASK_LOCK()/portGET_ISR_LOCK() and portRELEASE_TASK_LOCK()/portRELEASE_ISR_LOCK(). On some SMP ports (e.g. the Cortex-M7 and Cortex-R5 port from NXP), these locks are implemented using software spinlocks that use the macro portGET_CORE_ID() to determine the ID of the currently executing core.

On these same ports, portGET_CORE_ID() requires reading from peripheral registers that have non-trivial (and sometimes non-deterministic) access times. This impacts the overall execution time of certain critical functions and increases the overhead caused by FreeRTOS in SMP mode. Some examples include:

• vTaskSwitchContext(), which calls portGET_CORE_ID() once internally, but in the worst case 1 additional time per call to portGET_TASK_LOCK()/portGET_ISR_LOCK() and portRELEASE_TASK_LOCK()/portRELEASE_ISR_LOCK() depending on the port (up to 5 calls in total).
• vTaskEnterCritical() and vTaskExitCritical() which in the worst case each require 2 calls to portGET_CORE_ID() (one to release each lock).

These functions are used frequently by the scheduler so would benefit the most from optimization.

Describe the solution you'd like
The number of calls to portGET_CORE_ID() could be potentially reduced in the aforementioned functions by storing the core ID in a local variable and passing it as an argument to portGET_TASK_LOCK()/portGET_ISR_LOCK() and portRELEASE_TASK_LOCK()/portRELEASE_ISR_LOCK().

On my particular hardware (an NXP S32K3-series MCU with a Cortex-M7), a read of the peripheral register containing the core ID takes ~200-600ns each time, so a potential speedup of ~400-1,200ns could be possible with each get/take lock function call.

Describe alternatives you've considered
None so far.

How many devices will this feature impact?
Potentially all MCUs that must read the core ID from a register that is not local to the core itself (i.e, from a memory-mapped peripheral).

What are your project timelines?
ASAP 🙃

Additional context
None

[felixvanoost]

I realised that portGET_CORE_ID() is actually used in the CRITICAL_NESTING_COUNT macros in tasks.c as well, resulting in an additional function call each time. I've summed the number of (what I believe to be) unnecessary calls to portGET_CORE_ID() in the worst case for some of the most used functions:

Function name	Unnecessary calls to portGET_CORE_ID()
prvCheckForStateRunChange()	8
prvYieldForTask()	1
vTaskSuspendAll()	3
vTaskResumeAll()	1
vTaskSwitchContext()	5
vTaskYieldWithinAPI()	1
vTaskEnterCritical()	4
vTaskEnterCriticalFromISR()	2
vTaskExitCritical()	6
vTaskExitCriticalFromISR()	4

In my initial testing with storing the core ID in a local variable per function call, I was able to reduce the execution time of some of these functions by ~30% on my system.

I can submit a PR to resolve at least part of these if that would be helpful!

[amazonKamath]

Context: https://forums.freertos.org/t/smp-performance-benchmarking/22047/6

[felixvanoost]

I've opened a PR (#1206) to reduce the number of calls to portGET_CORE_ID() in the CRITICAL_NESTING_COUNT macros.

The changes I described in the initial issue would result in an additional execution time reduction but would require the port API to change slightly. The macros port{GET/RELEASE}_ISR_LOCK and port{GET/RELEASE}_TASK_LOCK would need to be updated to pass the core ID as a parameter, which will affect existing SMP ports.

[chinglee-iot]

Thank you for creating this PR. I will look into the detail and discuss with you in the PR.

[felixvanoost]

@chinglee-iot After #1206 is merged, the second part of my proposal in this issue is to also pass the core ID into the lock functions. This would require a breaking change to add the argument to the following macros:

portGET_TASK_LOCK()
portGET_ISR_LOCK()
portRELEASE_TASK_LOCK()
portRELEASE_ISR_LOCK()

This would impact the existing SMP ports. All the public SMP ports I've found currently need to get the core ID locally within their spinlock functions, so they would all benefit from this change.

The additional performance improvement would be similar in magnitude to #1206, as most of the same FreeRTOS functions get and/or release one or both locks. The benefit would be especially significant in vTaskEnterCritical()/vTaskExitCritical().

Is this something that the FreeRTOS team would be receptive to?