FOME-Tech / fome-fw

Free Open Motorsports ECU
https://www.fome.tech
Other
43 stars 22 forks source link

adding assertListIsSorted coverage to simulator #336

Closed rusefillc closed 8 months ago

mck1117 commented 8 months ago

Can we quantify the performance impact of this change? Those were guarded out for a reason, and that reason is that this is an extremely hot execution path. Given that we've never ever seen one of those assertions fail, I'm not sure this is worth the performance cost of adding the checks.

rusefillc commented 8 months ago

Can we quantify the performance impact of this change?

Why would anyone care for simulator performance impact? Whatever is simulator performance impact it's justified by additional coverage of more non-deterministic simulator.

mck1117 commented 8 months ago

simulator

ah, missed that one word.

And it seems the compiler in the firmware case does completely remove the empty calls, so zero hit there.