GENIVI / CANdb

Library for parsing CAN bus database description formats
Mozilla Public License 2.0
144 stars 46 forks source link

[embedded] Migration to PegLib C++17 for improving parsing performance? #39

Closed enricop closed 3 years ago

enricop commented 3 years ago

Hello,

we would like understand more about parsing performances of CANdb, and where cpu-time is actually spent when parsing a DBC.

In our application we are loading 2 DBC files:

here is the perf flamegraph of the first 40seconds of the whole applications startup.

parsing flamegraph

We have many modules in the application, however DBC parsing, in particular peglib.h codepath, covers almost 50% of all startup cpu-time.

We tried to switch the peglib submodule to the master branch, and made some quick adjustments to compile with std::string_view support. However the CANdb parsing is entering an infinite loop...we are investigating further.

Do you think that, for improving the parsing performance on an iMX6 embedded system, this is the way to go?

thanks for your support

enricop commented 3 years ago

we were able to modify the dbcparser.cpp to support peglib c++17. These are the durations for parsing the files on Desktop:

CAN1.dbc loaded in = 689[ms] CAN2.dbc loaded in = 1177[ms]

they are reduced by 30%

enricop commented 3 years ago

hello, by reverting this commit, which increases "cpp-peglib" submodule commit version, the performances from 20/30 seconds came back to 2/3 seconds on ARM iMX6 embedded systems.

going back to this v1.3.7 cpp-peglib commit the parsing time is the same on ARM systems as x86 systems.

btaczala commented 3 years ago

Hi @enricop, thanks a lot for great performance feedback. I think for now we can mitigate by going back to https://github.com/yhirose/cpp-peglib/commit/11ed83e, let's see how cpp-peglib maintainers react to https://github.com/yhirose/cpp-peglib/issues/173. In the mean time could you create a PR with going back to v1.3.7?

btaczala commented 3 years ago

OK, I forgot that we're on cpp-peglib v0.1.14. I'll try to bump it to v1.3.7 then

btaczala commented 3 years ago

I'll also add some benchmark tests and intergrate it with CI to track other performance degradations

btaczala commented 3 years ago

@enricop Does #40 fixes your performance problem?

enricop commented 3 years ago

hello, thanks for updating the cpp-peglib. I've tested on our new ARM A15 hardware and the problem I had on ARM A9 does not reproduce. If possible you can merge on master. thank you.