From 5068e41488e71499265e1fc3ea7d4210821e09a2 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 6 Mar 2025 13:10:39 -1000 Subject: [PATCH] feat: improve performance of signature lookups (#412) - Added `SignatureTree` type in a few more places to avoid python lookups - Improved performance and reduced code to create `Variant` object --- src/dbus_fast/_private/unmarshaller.pxd | 13 +++-- src/dbus_fast/_private/unmarshaller.py | 76 ++++++++++--------------- src/dbus_fast/message.pxd | 17 +++--- src/dbus_fast/message.py | 62 ++++++++++++-------- src/dbus_fast/signature.pxd | 6 +- src/dbus_fast/signature.py | 23 +++++--- 6 files changed, 103 insertions(+), 94 deletions(-) diff --git a/src/dbus_fast/_private/unmarshaller.pxd b/src/dbus_fast/_private/unmarshaller.pxd index bdd9512..6b4e696 100644 --- a/src/dbus_fast/_private/unmarshaller.pxd +++ b/src/dbus_fast/_private/unmarshaller.pxd @@ -60,6 +60,7 @@ cdef cython.dict MESSAGE_TYPE_MAP cdef cython.dict MESSAGE_FLAG_MAP cdef dict HEADER_MESSAGE_ARG_NAME + cdef SignatureTree SIGNATURE_TREE_EMPTY cdef SignatureTree SIGNATURE_TREE_B cdef SignatureTree SIGNATURE_TREE_N @@ -88,6 +89,7 @@ cdef SignatureType SIGNATURE_TREE_A_QV_TYPES_0 cdef SignatureTree SIGNATURE_TREE_A_OA_SA_SV cdef SignatureType SIGNATURE_TREE_A_OA_SA_SV_TYPES_0 + cdef unsigned int TOKEN_B_AS_INT cdef unsigned int TOKEN_U_AS_INT cdef unsigned int TOKEN_Y_AS_INT @@ -124,7 +126,7 @@ cdef unsigned short _ustr_uint16(const unsigned char * buf, unsigned int offset, cdef class Unmarshaller: - cdef object _unix_fds + cdef list _unix_fds cdef bytearray _buf cdef Py_ssize_t _buf_len cdef const unsigned char * _buf_ustr @@ -233,10 +235,11 @@ cdef class Unmarshaller: cdef _read_header(self) @cython.locals( - body=cython.list, - header_fields=cython.list, - token_as_int=cython.uint, - signature=cython.str, + body=list, + header_fields=list, + token_as_int="unsigned int", + signature=str, + tree=SignatureTree, message=Message ) cdef _read_body(self) diff --git a/src/dbus_fast/_private/unmarshaller.py b/src/dbus_fast/_private/unmarshaller.py index 304eef7..005626a 100644 --- a/src/dbus_fast/_private/unmarshaller.py +++ b/src/dbus_fast/_private/unmarshaller.py @@ -66,6 +66,8 @@ HEADER_SIGNATURE_SIZE = 16 HEADER_ARRAY_OF_STRUCT_SIGNATURE_POSITION = 12 +# Most common signatures + SIGNATURE_TREE_EMPTY = get_signature_tree("") SIGNATURE_TREE_B = get_signature_tree("b") SIGNATURE_TREE_N = get_signature_tree("n") @@ -76,19 +78,19 @@ SIGNATURE_TREE_Y = get_signature_tree("y") SIGNATURE_TREE_AY = get_signature_tree("ay") SIGNATURE_TREE_AS = get_signature_tree("as") -SIGNATURE_TREE_AS_TYPES_0 = SIGNATURE_TREE_AS.types[0] +SIGNATURE_TREE_AS_TYPES_0 = SIGNATURE_TREE_AS.root_type SIGNATURE_TREE_A_SV = get_signature_tree("a{sv}") -SIGNATURE_TREE_A_SV_TYPES_0 = SIGNATURE_TREE_A_SV.types[0] +SIGNATURE_TREE_A_SV_TYPES_0 = SIGNATURE_TREE_A_SV.root_type SIGNATURE_TREE_AO = get_signature_tree("ao") -SIGNATURE_TREE_AO_TYPES_0 = SIGNATURE_TREE_AO.types[0] +SIGNATURE_TREE_AO_TYPES_0 = SIGNATURE_TREE_AO.root_type SIGNATURE_TREE_OAS = get_signature_tree("oas") SIGNATURE_TREE_OAS_TYPES_1 = SIGNATURE_TREE_OAS.types[1] -SIGNATURE_TREE_AY_TYPES_0 = SIGNATURE_TREE_AY.types[0] +SIGNATURE_TREE_AY_TYPES_0 = SIGNATURE_TREE_AY.root_type SIGNATURE_TREE_A_QV = get_signature_tree("a{qv}") -SIGNATURE_TREE_A_QV_TYPES_0 = SIGNATURE_TREE_A_QV.types[0] +SIGNATURE_TREE_A_QV_TYPES_0 = SIGNATURE_TREE_A_QV.root_type SIGNATURE_TREE_SA_SV_AS = get_signature_tree("sa{sv}as") SIGNATURE_TREE_SA_SV_AS_TYPES_1 = SIGNATURE_TREE_SA_SV_AS.types[1] @@ -98,7 +100,8 @@ SIGNATURE_TREE_OA_SA_SV = get_signature_tree("oa{sa{sv}}") SIGNATURE_TREE_OA_SA_SV_TYPES_1 = SIGNATURE_TREE_OA_SA_SV.types[1] SIGNATURE_TREE_A_OA_SA_SV = get_signature_tree("a{oa{sa{sv}}}") -SIGNATURE_TREE_A_OA_SA_SV_TYPES_0 = SIGNATURE_TREE_A_OA_SA_SV.types[0] +SIGNATURE_TREE_A_OA_SA_SV_TYPES_0 = SIGNATURE_TREE_A_OA_SA_SV.root_type + TOKEN_B_AS_INT = ord("b") TOKEN_U_AS_INT = ord("u") @@ -529,66 +532,47 @@ class Unmarshaller: def _read_variant(self) -> Variant: signature = self._read_signature() token_as_int = ord(signature[0]) - var = Variant.__new__(Variant) # verify in Variant is only useful on construction not unmarshalling if len(signature) == 1: if token_as_int == TOKEN_N_AS_INT: - var._init_variant(SIGNATURE_TREE_N, self._read_int16_unpack(), False) - return var + return Variant._factory(SIGNATURE_TREE_N, self._read_int16_unpack()) if token_as_int == TOKEN_S_AS_INT: - var._init_variant(SIGNATURE_TREE_S, self._read_string_unpack(), False) - return var + return Variant._factory(SIGNATURE_TREE_S, self._read_string_unpack()) if token_as_int == TOKEN_B_AS_INT: - var._init_variant(SIGNATURE_TREE_B, self._read_boolean(), False) - return var + return Variant._factory(SIGNATURE_TREE_B, self._read_boolean()) if token_as_int == TOKEN_O_AS_INT: - var._init_variant(SIGNATURE_TREE_O, self._read_string_unpack(), False) - return var + return Variant._factory(SIGNATURE_TREE_O, self._read_string_unpack()) if token_as_int == TOKEN_U_AS_INT: - var._init_variant(SIGNATURE_TREE_U, self._read_uint32_unpack(), False) - return var + return Variant._factory(SIGNATURE_TREE_U, self._read_uint32_unpack()) if token_as_int == TOKEN_Y_AS_INT: self._pos += 1 - var._init_variant(SIGNATURE_TREE_Y, self._buf[self._pos - 1], False) - return var + return Variant._factory(SIGNATURE_TREE_Y, self._buf[self._pos - 1]) elif token_as_int == TOKEN_A_AS_INT: if signature == "ay": - var._init_variant( - SIGNATURE_TREE_AY, self.read_array(SIGNATURE_TREE_AY_TYPES_0), False + return Variant._factory( + SIGNATURE_TREE_AY, self.read_array(SIGNATURE_TREE_AY_TYPES_0) ) - return var if signature == "a{qv}": - var._init_variant( - SIGNATURE_TREE_A_QV, - self.read_array(SIGNATURE_TREE_A_QV_TYPES_0), - False, + return Variant._factory( + SIGNATURE_TREE_A_QV, self.read_array(SIGNATURE_TREE_A_QV_TYPES_0) ) - return var if signature == "as": - var._init_variant( - SIGNATURE_TREE_AS, self.read_array(SIGNATURE_TREE_AS_TYPES_0), False + return Variant._factory( + SIGNATURE_TREE_AS, self.read_array(SIGNATURE_TREE_AS_TYPES_0) ) - return var if signature == "a{sv}": - var._init_variant( - SIGNATURE_TREE_A_SV, - self.read_array(SIGNATURE_TREE_A_SV_TYPES_0), - False, + return Variant._factory( + SIGNATURE_TREE_A_SV, self.read_array(SIGNATURE_TREE_A_SV_TYPES_0) ) - return var if signature == "ao": - var._init_variant( - SIGNATURE_TREE_AO, self.read_array(SIGNATURE_TREE_AO_TYPES_0), False + return Variant._factory( + SIGNATURE_TREE_AO, self.read_array(SIGNATURE_TREE_AO_TYPES_0) ) - return var tree = get_signature_tree(signature) - signature_type = tree.types[0] - var._init_variant( - tree, - self._readers[signature_type.token](self, signature_type), - False, + signature_type = tree.root_type + return Variant._factory( + tree, self._readers[signature_type.token](self, signature_type) ) - return var def read_struct(self, type_: _SignatureType) -> list[Any]: self._pos += -self._pos & 7 # align 8 @@ -725,7 +709,7 @@ class Unmarshaller: # There shouldn't be any other types in the header # but just in case, we'll read it using the slow path headers[field_0] = self._readers[token]( - self, get_signature_tree(token).types[0] + self, get_signature_tree(token).root_type ) return headers @@ -780,7 +764,7 @@ class Unmarshaller: self._pos = HEADER_ARRAY_OF_STRUCT_SIGNATURE_POSITION header_fields = self._header_fields(self._header_len) self._pos += -self._pos & 7 # align 8 - signature = header_fields[HEADER_SIGNATURE_IDX] + signature: str = header_fields[HEADER_SIGNATURE_IDX] if not self._body_len: tree = SIGNATURE_TREE_EMPTY body: list[Any] = [] diff --git a/src/dbus_fast/message.pxd b/src/dbus_fast/message.pxd index dd33860..d453144 100644 --- a/src/dbus_fast/message.pxd +++ b/src/dbus_fast/message.pxd @@ -3,15 +3,11 @@ import cython from ._private.marshaller cimport Marshaller -from .signature cimport Variant - +from .signature cimport Variant, SignatureTree, SignatureType cdef object ErrorType -cdef object SignatureTree -cdef object SignatureType cdef object MessageType - cdef object HEADER_PATH cdef object HEADER_INTERFACE cdef object HEADER_MEMBER @@ -30,6 +26,11 @@ cdef object MESSAGE_FLAG cdef object MESSAGE_FLAG_NONE cdef object MESSAGE_TYPE_METHOD_CALL +cdef SignatureTree SIGNATURE_TREE_G +cdef SignatureTree SIGNATURE_TREE_O +cdef SignatureTree SIGNATURE_TREE_S +cdef SignatureTree SIGNATURE_TREE_U + cdef get_signature_tree cdef class Message: @@ -44,8 +45,8 @@ cdef class Message: cdef public unsigned int reply_serial cdef public object sender cdef public list unix_fds - cdef public object signature - cdef public object signature_tree + cdef public str signature + cdef public SignatureTree signature_tree cdef public object body cdef public unsigned int serial @@ -68,7 +69,7 @@ cdef class Message: unsigned int reply_serial, object sender, list unix_fds, - object signature_tree, + SignatureTree signature_tree, object body, unsigned int serial, bint validate diff --git a/src/dbus_fast/message.py b/src/dbus_fast/message.py index d5ea526..e701d0f 100644 --- a/src/dbus_fast/message.py +++ b/src/dbus_fast/message.py @@ -33,6 +33,10 @@ MESSAGE_FLAG = MessageFlag MESSAGE_FLAG_NONE = MessageFlag.NONE MESSAGE_TYPE_METHOD_CALL = MessageType.METHOD_CALL +SIGNATURE_TREE_G = get_signature_tree("g") +SIGNATURE_TREE_O = get_signature_tree("o") +SIGNATURE_TREE_S = get_signature_tree("s") +SIGNATURE_TREE_U = get_signature_tree("u") _int = int _str = str @@ -325,37 +329,47 @@ class Message: # Variant is invalid. if self.path: - var = Variant.__new__(Variant) - var._init_variant("o", self.path, False) - fields.append((HEADER_PATH, var)) + fields.append((HEADER_PATH, Variant._factory(SIGNATURE_TREE_O, self.path))) if self.interface: - var = Variant.__new__(Variant) - var._init_variant("s", self.interface, False) - fields.append((HEADER_INTERFACE, var)) + fields.append( + (HEADER_INTERFACE, Variant._factory(SIGNATURE_TREE_S, self.interface)) + ) if self.member: - var = Variant.__new__(Variant) - var._init_variant("s", self.member, False) - fields.append((HEADER_MEMBER, var)) + fields.append( + (HEADER_MEMBER, Variant._factory(SIGNATURE_TREE_S, self.member)) + ) if self.error_name: - var = Variant.__new__(Variant) - var._init_variant("s", self.error_name, False) - fields.append((HEADER_ERROR_NAME, var)) + fields.append( + ( + HEADER_ERROR_NAME, + Variant._factory(SIGNATURE_TREE_S, self.error_name), + ) + ) if self.reply_serial: - var = Variant.__new__(Variant) - var._init_variant("u", self.reply_serial, False) - fields.append((HEADER_REPLY_SERIAL, var)) + fields.append( + ( + HEADER_REPLY_SERIAL, + Variant._factory(SIGNATURE_TREE_U, self.reply_serial), + ) + ) if self.destination: - var = Variant.__new__(Variant) - var._init_variant("s", self.destination, False) - fields.append((HEADER_DESTINATION, var)) + fields.append( + ( + HEADER_DESTINATION, + Variant._factory(SIGNATURE_TREE_S, self.destination), + ) + ) if self.signature: - var = Variant.__new__(Variant) - var._init_variant("g", self.signature, False) - fields.append((HEADER_SIGNATURE, var)) + fields.append( + (HEADER_SIGNATURE, Variant._factory(SIGNATURE_TREE_G, self.signature)) + ) if self.unix_fds and negotiate_unix_fd: - var = Variant.__new__(Variant) - var._init_variant("u", len(self.unix_fds), False) - fields.append((HEADER_UNIX_FDS, var)) + fields.append( + ( + HEADER_UNIX_FDS, + Variant._factory(SIGNATURE_TREE_U, len(self.unix_fds)), + ) + ) header_body = [ LITTLE_ENDIAN, diff --git a/src/dbus_fast/signature.pxd b/src/dbus_fast/signature.pxd index ea125cd..eaf94e2 100644 --- a/src/dbus_fast/signature.pxd +++ b/src/dbus_fast/signature.pxd @@ -15,6 +15,7 @@ cdef class SignatureTree: cdef public str signature cdef public list types + cdef public SignatureType root_type cdef class Variant: @@ -23,5 +24,6 @@ cdef class Variant: cdef public str signature cdef public object value - @cython.locals(signature_tree=SignatureTree) - cdef _init_variant(self, object signature, object value, bint verify) + @cython.locals(self=Variant) + @staticmethod + cdef Variant _factory(SignatureTree signature_tree, object value) diff --git a/src/dbus_fast/signature.py b/src/dbus_fast/signature.py index 07308ba..bfc0829 100644 --- a/src/dbus_fast/signature.py +++ b/src/dbus_fast/signature.py @@ -328,11 +328,14 @@ class SignatureTree: :ivar ~.signature: The signature of this signature tree. :vartype ~.signature: str + :ivar root_type: The root type of this signature tree. + :vartype root_type: :class:`SignatureType + :raises: :class:`InvalidSignatureError` if the given signature is not valid. """ - __slots__ = ("signature", "types") + __slots__ = ("root_type", "signature", "types") def __init__(self, signature: str = "") -> None: self.signature = signature @@ -346,6 +349,8 @@ class SignatureTree: (type_, signature) = SignatureType._parse_next(signature) self.types.append(type_) + self.root_type = self.types[0] if self.types else None + def __eq__(self, other: object) -> bool: if type(other) is SignatureTree: return self.signature == other.signature @@ -403,14 +408,6 @@ class Variant: verify: bool = True, ) -> None: """Init a new Variant.""" - self._init_variant(signature, value, verify) - - def _init_variant( - self, - signature: Union[str, SignatureTree, SignatureType], - value: Any, - verify: bool, - ) -> None: if type(signature) is SignatureTree: signature_tree = signature self.signature = signature_tree.signature @@ -435,6 +432,14 @@ class Variant: ) self.type.verify(value) + @staticmethod + def _factory(signature_tree: SignatureTree, value: Any) -> "Variant": + self = Variant.__new__(Variant) + self.signature = signature_tree.signature + self.type = signature_tree.root_type + self.value = value + return self + def __eq__(self, other: object) -> bool: if type(other) is Variant: return self.signature == other.signature and self.value == other.value