⚡️ zv,zn,zb,zd,zm: Add zvariant::parsed::Signature & use that in ser/de #966
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Let's move the main and mandatory dependencies to the top of the file.
We'll be using this for our new signal parsing code.
This is a generic variant for invalid signatures. It will be used in a following commit when parsing the new signature type.
The internal ser/de structs don't need to implement `Send`, `Sync` or `Unpin`.
This is a new type that represents a parsed signature. We'll use this to avoid parsing the signature as part of (de)serialization, which seems to be quite expensive at times, especially for large arrays. While the the parsed::Signature can be constructed w/o any allocation and in const context even, unfortunately converting from and to `Signature` or the underlying string form, is not free and will require allocations for non-basic types. However, we'll be changing `Type` trait to provide us a `parsed::Signature` as a constant, allowing us to avoid allocations and parsing.
* Th Serializer impls now use parsed::Signature. * We add new serialization functions that that take a parsed::Signature. * We keep backwards-compatibility by keeping the existing serialization functions that translate Signature to parsed::Signature. This is not going to be as efficient as using the parsed::Signature from the beginning but we'll be moving towards using the parsed::Signature in the following commits anyway. Benchmarks show that this change can result is big performance gains, ranging from 5% to 94%, depending on the type being encoded and the host system resources available (and other environmental factors).
zeenix
force-pushed
the
parsed-signature
branch
2 times, most recently
from
0105b80
to
03a25db
Compare
* Th Deserializer impls now use parsed::Signature. * We add new serialized::Data::deserialize_for_parsed_signature method. * We keep backwards-compatibility by keeping the existing serialized::Data::deserialize method that translate Signature to parsed::Signature. This is not going to be as efficient as using the parsed::Signature from the beginning but we'll be moving towards using the parsed::Signature in the following commits anyway.
This already shows off the performance gains, proving that a parsed signature is really the way to go. In the following commits, we'll also update the Type to give us parsed signature so we can have ser/de optimized e2e.
Add parsed signature consts to `Basic`. This doesn't break the API since we can determine the signature in const context, using `Basic::SIGNATURE_CHAR`.
Add a method to `Type` trait that returns the parsed form of the signature. This method has a default implementation to not break the API. By default, it parses the return value of the `signature()` method. Since that is not efficient at all, we encourage implementors to implement this method (instead of `signature()`). Moreover, for keeping type implementation easy, we also now provide a default implementation of `signature`, which converts the parsed signature back to a string form. This does indeed create a mutual recursion between the default implementations of `signature` and `parsed_signature`, but I believe this is a good trade-off for the sake of simplicity and ease of implementation. All current manul implementations of `Type` out there are guaranteed to have an implementation of `signature` method, so this will only affect new implementations. Hopefully they will read the docs when they implement the trait. If not, they'll quickly run into a stack overflow, which will force them to read the docs. :) This commit also replaces most implementations of `signature` with `parsed_signature`. Only manual implementations of `Type` outside this repository will want to (not need to) implement `parsed_signature` if they want encoding/decoding to be super efficient efficient. I originally wrote this as a const (as it should be) but that will need to have a default value to not break the API and from a consts context, we can't call `signature`, even if we could parse its return value. This will be an easy fix to add in the next major version.
This is analogous to the `Type::parsed_signature` method, but for dynamic types.
This is just a natural follow-up to the addition of `DynamicDeserialize::dynamic_parsed_signature` method.
A variant of `deserialize_for_dynamic_signature` that takes a parsed signature reference.
Instead of first getting the signature and then calling specific ser/de API that takes a signature.
Instead make use of the new `parsed::Signature` API. This involves breaking the API a bit since now both `ArraySeed` and `StructureSeed` do not implement `Sync` and `Send` now. However the chances of anyong using them directly are already extremely low and even if they'd use them directly, the changes of them needing `Sync` and `Send` are even lower. So I think we'll be ok.
It's no longer used or needed. So long!
Otherwise, it can enable Maybe singature support, which will fail with an assertion when used with dbus ser/de.
zeenix
force-pushed
the
parsed-signature
branch
from
03a25db
to
54a8462
Compare
This was referenced Sep 2, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The commits in the PR are quite self-explanatory but the main benefit coming from this PR, is massive efficiency gains in ser/de of data. Benchmarks reveal that on average we gain about 50% improvement in performance. The fixed-array ser benchmark can now be up to 94% faster on some machines. On my (rather high-end) machine, the benchmark results are as follows:
This PR addresses most of #882. Completely addressing it would require breaking API so it has to wait for the next major release.