diff --git a/can/interfaces/socketcan/socketcan.py b/can/interfaces/socketcan/socketcan.py index 6dc856cbf..eaced1e0b 100644 --- a/can/interfaces/socketcan/socketcan.py +++ b/can/interfaces/socketcan/socketcan.py @@ -47,6 +47,8 @@ CMSG_SPACE(RECEIVED_TIMESTAMP_STRUCT.size) if CMSG_SPACE_available else 0 ) +MSG_DONTROUTE = int(socket.MSG_DONTROUTE) + # Setup BCM struct def bcm_header_factory( @@ -656,7 +658,7 @@ def capture_message(sock: socket.socket, get_channel: bool = False) -> Message | error_state_indicator = bool(flags & constants.CANFD_ESI) # Section 4.7.1: MSG_DONTROUTE: set when the received frame was created on the local host. - is_rx = not bool(msg_flags & socket.MSG_DONTROUTE) + is_rx = not msg_flags & MSG_DONTROUTE if is_extended_frame_format: # log.debug("CAN: Extended") @@ -666,23 +668,22 @@ def capture_message(sock: socket.socket, get_channel: bool = False) -> Message | # log.debug("CAN: Standard") arbitration_id = can_id & 0x000007FF - msg = Message( - timestamp=timestamp, - channel=channel, - arbitration_id=arbitration_id, - is_extended_id=is_extended_frame_format, - is_remote_frame=is_remote_transmission_request, - is_error_frame=is_error_frame, - is_fd=is_fd, - is_rx=is_rx, - bitrate_switch=bitrate_switch, - error_state_indicator=error_state_indicator, - dlc=can_dlc, - data=data, + # Built positionally: binding twelve keyword arguments is slower. + return Message( + timestamp, + arbitration_id, + is_extended_frame_format, + is_remote_transmission_request, + is_error_frame, + channel, + can_dlc, + data, + is_fd, + is_rx, + bitrate_switch, + error_state_indicator, ) - return msg - class SocketcanBus(BusABC): # pylint: disable=abstract-method """A SocketCAN interface to CAN. diff --git a/doc/changelog.d/2097.changed.rst b/doc/changelog.d/2097.changed.rst new file mode 100644 index 000000000..d709605de --- /dev/null +++ b/doc/changelog.d/2097.changed.rst @@ -0,0 +1 @@ +Improved performance of the ``socketcan`` receive path: ``capture_message`` now uses roughly a third less CPU per frame. diff --git a/test/test_socketcan.py b/test/test_socketcan.py index 9d042f425..771448619 100644 --- a/test/test_socketcan.py +++ b/test/test_socketcan.py @@ -5,6 +5,8 @@ """ import ctypes +import inspect +import socket import struct import sys import unittest @@ -20,6 +22,7 @@ TX_COUNTEVT, ) from can.interfaces.socketcan.socketcan import ( + MSG_DONTROUTE, BcmMsgHead, bcm_header_factory, build_bcm_header, @@ -391,5 +394,43 @@ def test_pypy_socketcan_support(self): ) +class SocketCANHotPathTest(unittest.TestCase): + """Guard the per-frame optimisations in :func:`capture_message`.""" + + def test_msg_dontroute_is_plain_int(self): + """``socket.MSG_DONTROUTE`` is an ``enum.IntFlag`` member on CPython, and + evaluating ``msg_flags & `` once per received frame constructs a new + flag instance each time. ``capture_message`` therefore tests against a plain + int, which must still carry the same value. + """ + self.assertEqual(MSG_DONTROUTE, socket.MSG_DONTROUTE) + self.assertIs(type(MSG_DONTROUTE), int) + + def test_capture_message_message_args(self): + """``capture_message`` builds its :class:`~can.Message` positionally to avoid + per-frame keyword binding, so a reordering of ``Message.__init__`` would + silently corrupt every received frame. Pin the expected order. + """ + params = list(inspect.signature(can.Message.__init__).parameters) + self.assertEqual( + params[:13], + [ + "self", + "timestamp", + "arbitration_id", + "is_extended_id", + "is_remote_frame", + "is_error_frame", + "channel", + "dlc", + "data", + "is_fd", + "is_rx", + "bitrate_switch", + "error_state_indicator", + ], + ) + + if __name__ == "__main__": unittest.main()