commaai / opendbc

democratize access to car decoder rings
MIT License
1.81k stars 1.07k forks source link

Refactor CAN Parser to boost performance #1046

Open deanlee opened 1 month ago

deanlee commented 1 month ago

Shift most logic to C++, read values directly from c++ MessageState instead of maintaining a copy in cython. Cython's update_string acts as a simple interface wrapper for the C++ function.


def update_strings(self, strings, sendcan=False):
    return self.can.update_strings(strings, sendcan)

master:

6000 CAN packets, 20 runs 293.75 mean ms, 295.81 max ms, 289.66 min ms, 1.42 std ms 0.0490 mean ms / CAN packet

this branch:

6000 CAN packets, 20 runs 29.34 mean ms, 33.17 max ms, 27.16 min ms, 1.29 std ms 0.0049 mean ms / CAN packet

This PR also resolves https://github.com/commaai/opendbc/issues/913. Since we no longer rely on calling update_strings in __init__ to initialize Cython members, update_strings() now returns an empty list if strings is empty.

After this PR, we can simplify car/interface by using only one CanParser. With a single call to update_string for parsing all messages, there's no need to repeatedly feed identical can_strings to multiple instances of CanParser's update_string method for message filtering. This straightforward approach greatly enhances car/interface and parsing efficiency.

Additional Notes:

PlotJuggler will be broken after this PR because data is now read directly from MessageState instead of being copied using query_latest. However, it is quite easy to modify PlotJuggler to use the new API.

sshane commented 1 month ago

:eyes:

batman@workstation-shane:~/openpilot/selfdrive/debug$ ./check_can_parser_performance.py 
100%|██████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 10/10 [00:01<00:00,  7.92it/s]
6000 CAN packets, 10 runs
99.36 mean ms, 99.86 max ms, 98.82 min ms, 0.33 std ms
0.0166 mean ms / CAN packet
batman@workstation-shane:~/openpilot/selfdrive/debug$ ./check_can_parser_performance.py 
100%|██████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 10/10 [00:00<00:00, 20.73it/s]
6000 CAN packets, 10 runs
23.19 mean ms, 23.77 max ms, 22.84 min ms, 0.25 std ms
0.0039 mean ms / CAN packet
deanlee commented 1 month ago

👀

batman@workstation-shane:~/openpilot/selfdrive/debug$ ./check_can_parser_performance.py 
100%|██████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 10/10 [00:01<00:00,  7.92it/s]
6000 CAN packets, 10 runs
99.36 mean ms, 99.86 max ms, 98.82 min ms, 0.33 std ms
0.0166 mean ms / CAN packet
batman@workstation-shane:~/openpilot/selfdrive/debug$ ./check_can_parser_performance.py 
100%|██████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 10/10 [00:00<00:00, 20.73it/s]
6000 CAN packets, 10 runs
23.19 mean ms, 23.77 max ms, 22.84 min ms, 0.25 std ms
0.0039 mean ms / CAN packet

Yes, this test should be accurate. The test results from my master version are outdated.

sshane commented 1 month ago

After this PR, we can simplify car/interface by using only one CanParser. With a single call to update_string for parsing all messages, there's no need to repeatedly feed identical can_strings to multiple instances of CanParser's update_string method for message filtering. This straightforward approach greatly enhances car/interface and parsing efficiency.

Nice, you mean we can pass in a dictionary with bus as the key, or similar?

sshane commented 1 month ago

On a comma 3X:

6000 CAN packets, 10 runs
300.33 mean ms, 302.04 max ms, 296.67 min ms, 1.42 std ms
0.0501 mean ms / CAN packet

vs.

6000 CAN packets, 10 runs
86.96 mean ms, 87.91 max ms, 86.36 min ms, 0.54 std ms
0.0145 mean ms / CAN packet
deanlee commented 1 month ago

After this PR, we can simplify car/interface by using only one CanParser. With a single call to update_string for parsing all messages, there's no need to repeatedly feed identical can_strings to multiple instances of CanParser's update_string method for message filtering. This straightforward approach greatly enhances car/interface and parsing efficiency.

Nice, you mean we can pass in a dictionary with bus as the key, or similar?

y, or may something like{bus: [(msg1, freq1), (msg2, freq2)...], bus2: [...]}