perf: optimize serialization of arrays of trivial D-Bus types

[pliniofpa]

Discussion #338 and Issue #339

Closes #338
Fixes #339

[sangelovic]

Hey @pliniofpa, this is surely an improvement and we will take this in. I've looked at your changes and have done adjustments on your branch. I simplified it. In essence, template specializations have been removed, instead constexpr if is used in vector functions where either fast path or slow path is taken depending on _Element traits. signature_of type trait class has been extended with is_trivial_dbus_type for that purpose. appendArray() and readArray() are now private and work with pointers directly. Clients simply serialize/deserialize vectors and the rest is hidden.

Additionally, I made all the insertion and extraction operators as member functions. Until now, some were member functions and some were free functions, I don't know why. Now 1. it's consistent, 2. all supported overloads are easily visible together in Message class definition, 3. it allows to make underlying appendArray() and readArray() functions as private, which they shall be. So a number of changes is due to that additional, not-that-much-related refactoring.

Feel free to review. Does it look OK to you? If so, let me know and we'll merge it.

Thank you for sharing your observation and submitting this PR!

[pliniofpa]

Stanislav, I like the idea of making the operators all members of the Message class. The changes look good to me you can go ahead with the merge. Off topic: Is there a reason why sdbus-cpp doesn't have members to/from std::array<>? Sincerely, Plinio Andrade

…

________________________________ From: Stanislav Angelovič ***@***.***> Sent: Thursday, July 27, 2023 8:10:34 AM To: Kistler-Group/sdbus-cpp ***@***.***> Cc: pliniofpa ***@***.***>; Mention ***@***.***> Subject: Re: [Kistler-Group/sdbus-cpp] Improve performance of std::vector<> per Issue #339. (PR #340) Hey @pliniofpa<https://github.com/pliniofpa>, this is surely an improvement and we will take this in. I've looked at your changes and have done adjustments on your branch. I simplified it. Instead of template specializations, I used constexpr if, plus a few other details. Additionally, I made all the insertion and extraction operators as member functions. Until now, some were member functions and some were free functions. So a number of changes is due to that additional refactoring. Feel free to review. Does it look OK to you? If so, let me know and we'll merge it. — Reply to this email directly, view it on GitHub<#340 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AA2LGGPUFEIVR7MCTYWT76LXSJLDVANCNFSM6AAAAAA2YIGCFU>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[pliniofpa]

This looks good to me.
Thanks.

[sangelovic]

@pliniofpa

Off topic:
Is there a reason why sdbus-cpp doesn't have members to/from std::array<>?

No reason. We can add it. There is actually a discussion going on with that proposal, among others, at #328.

The open question is what the behavior should be when the std::array size is lower or greater than the real number of elements when deserializing from the message. Will that be not allowed (an exception thrown)? Shall we populate only as many elements as std::array enables and throw away the rest? Shall we fill the array up until the number of elements in the message and leave the rest of elements in the array untouched?

Feel free to contribute, either in the discussion at #328 or by submitting a PR ;)

Thanks and take care.

[pliniofpa]

Stanislav, I contributed with a comment. Let me know if you agree. Sincerely, Plinio Andrade

…

________________________________ From: Stanislav Angelovič ***@***.***> Sent: Thursday, July 27, 2023 12:29:07 PM To: Kistler-Group/sdbus-cpp ***@***.***> Cc: pliniofpa ***@***.***>; Mention ***@***.***> Subject: Re: [Kistler-Group/sdbus-cpp] Improve performance of std::vector<> per Issue #339. (PR #340) Off topic: Is there a reason why sdbus-cpp doesn't have members to/from std::array<>? No reason. We can add it. There is actually a discussion going on with that proposal, among others, at #328<#328>. The open question is what the behavior should be when the std::array size is lower or greater than the real number of elements when deserializing from the message. Will that be not allowed (an exception thrown)? Shall we populate only as many elements as std::array enables and throw away the rest? Shall we fill the array up until the number of elements in the message and leave the rest of elements in the array untouched? Feel free to contribute, either in the discussion at #328<#328> or by submitting a PR ;) Thanks and take care. — Reply to this email directly, view it on GitHub<#340 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AA2LGGJIBRR2YJN3LR67ZHLXSKJNHANCNFSM6AAAAAA2YIGCFU>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[sangelovic]

@pliniofpa Hi Plinio, yes I agree with you, thanks, I've just replied in that thread.