extensive cpu on every parameter change

[BazookaJoe1900]

Describe the bug
On every parameter change there is extensive cpu use that can be reduce.

The reason is that on every parameter change there is a parameter_update.msg uorb.

some modules uses that as trigger to update their parameter.
some modules doesn't cache the handler of the required parameter. so they search it by param_find()
but param_find() is a bit heavy, it does a binary search of the name of the parameter on the list of all of the parameters.

I think that moving modules to the c++ param wrapper is advised.

[dagar]

Where do you see param_find() being called as a result of parameter_update publications?

[BazookaJoe1900]

after printing the parameters that being searched after another parameter is beeing called I can see 177 searches for parameters. (some are double search from different modules)
here is the list:
CAL_ACC*
CAL_GYRO*
CAL_MAG*
CBRK_AIRSPD_CHK
CBRK_ENGINEFAIL
CBRK_FLIGHTTERM
CBRK_GPSFAIL
CBRK_IO_SAFETY
CBRK_SUPPLY_CHK
CBRK_USB_CHK
FW_MAN*
PWM_MAIN*
PWM_SBUS_MODE
RC_FAILS_THR
RC_MAP_AUX1
RC_RSSI_PWM_*
RC*
SENS_EN_THERMAL
TC_*
THR_MDL_FAC
TRIM*

[dagar]

Great, so the next step is to track down the offending modules and either store the param_t value, or better yet move to module parameters (px4_module.h) where possible.

Example
https://github.com/PX4/Firmware/blob/6f3868b5ba75de38a409c2ad72251dfaaee99a3e/src/modules/mc_att_control/mc_att_control.hpp#L209-L268

[BazookaJoe1900]

I can do that, at least some... Just wanted to be sure it can be consider as something to fix before I implement a fix

[dagar]

Yes, virtually all param_find() after boot is complete should be eliminated. If you take a look at the perf counter (run perf) you'll see counters for param_find, param_get, etc.

param_set: 6 events, 27us elapsed, 4.50us avg, min 2us max 13us 84.218us rms
param_get: 1142 events, 6547us elapsed, 5.73us avg, min 2us max 572us 68.706us rms
param_find: 116 events, 651us elapsed, 5.61us avg, min 2us max 143us 48.639us rms
param_export: 1 events, 45960us elapsed, 45960.00us avg, min 45960us max 45960us   infus rms

[BazookaJoe1900]

also

param status                                                                                            
INFO  [parameters] summary: 727/1118 (used/total)                                                       
INFO  [parameters] file: /fs/mtd_params                                                                 
INFO  [parameters] storage array: 16/16 elements (256 bytes total)                                      
INFO  [parameters] auto save: on                                                                        
INFO  [parameters] last auto save: 1065.471 seconds ago                                                 
param_export: 1 events, 6118us elapsed, 6118.00us avg, min 6118us max 6118us   nanus rms                
param_find: 176 events, 60618us elapsed, 344.42us avg, min 10us max 5533us 4014.146us rms               
param_get: 712 events, 2723us elapsed, 3.82us avg, min 2us max 231us 37.864us rms                       
param_set: 1 events, 11us elapsed, 11.00us avg, min 11us max 11us   infus rms                           
nsh> WARN  [load_mon] wq:manager low on stack! (276 bytes left)              

[dagar]

With #13000 merged what's left?

[BazookaJoe1900]

@dagar @julianoes
there are many more, you can see them on the 3rd comment.
most of them are more 'difficult' to solve because they are more 'spacial cases'.
What I spotted as problematic is:

• CAL_, TC_- The name of the parameters is build on runtime. If you want I can change this behavior. I didn't want to do that before I understand you agree its an issue....

• FW_MAN_, TRIM_ - raised a real issue, as written on [WIP] remove the use of param_find("") #13076

• PWM_MAIN_*, PWM_SBUS_MODE, and others - they are on the px4io, do you see any problem to add the DEFINE_PARAMETERS() to the px4io?

There might be others parameters, but its a bit hard to find them among the long list, the best is to solve one by one and find the problematic cases.

[BazookaJoe1900]

In general, do you see cases that we really need param_find() by param name? do you see cases that code will need to search parameter and it won't be found?
#13076 shows a case that there must not be a call to parameter, it doesn't exists....

[dagar]

In general, do you see cases that we really need param_find() by param name? do you see cases that code will need to search parameter and it won't be found?

Only a few, and those should be the exception that would ultimately be replaced architecturally.

For example, commander shouldn't be dependent on any vehicle specific parameters. PX4 should be modular to the point it can be stripped down (customized) for a single vehicle type.

https://github.com/PX4/Firmware/blob/d537f3ec6ba2a58188869481f1dbcd36dc447a36/src/modules/commander/PreflightCheck.cpp#L917

https://github.com/PX4/Firmware/blob/d537f3ec6ba2a58188869481f1dbcd36dc447a36/src/modules/commander/Commander.cpp#L1240

[stale]

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.