Redesign serialization classes #975
Merged
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
- Remove `data_processor`. This class unified serializer and deserializer interfaces to reduce code duplication. However, the implementation is quite complex and enforces virtual dispatch for all derived types. - Add `write_inspector` and `read_inspector` utility classes that inject an `operator()`. The previous recursive argument unrolling in the `data_processor` no longer has its place in C++17. The two new classes dispatch on `apply` member functions of their subtype via CRTP and use `if constexpr` to dispatch to the correct functions. - Refactor `serializer` and `deserializer` classes. These two classes still serve as base types for abstract inspectors, but no longer are connected through a common base. - Add `binary_serializer` and `binary_deserializer` classes. These two classes are the workhorses in CAF that run in performance-critical code. Hence, these implementations must reduce overhead as much as possible. Because of this, they do not inherit from the abstract interfaces and internally use the new `error_code` class over the generic `error` class. - Add handlers for `binary_serializer` and `binary_deserializer` to all CAF classes in addition to the generic versions.
josephnoir
reviewed
Nov 22, 2019
|
|
||
| instance(abstract_broker* parent, callee& lstnr); | ||
|
|
||
| /// Handles received data and returns a config for receiving the | ||
| /// next data or `none` if an error occurred. | ||
| /// next data or `none` if an error occured. |
There was a problem hiding this comment.
Conflict with that spelling PR, i guess.
Suggested change
| /// next data or `none` if an error occured. | |
| /// Next data or `none` if an error occurred. |
| @@ -352,6 +366,11 @@ void middleman::init(actor_system_config& cfg) { | |||
| return sec::no_such_group_module; | |||
| } | |||
|
|
|||
| error_code<sec> load(binary_deserializer&, group&) override { | |||
| // never called, because we hand out group instances of the local module | |||
There was a problem hiding this comment.
Suggested change
| // never called, because we hand out group instances of the local module | |
| // Never called, because we hand out group instances of the local module. |
Co-Authored-By: Joseph Noir <josephnoir@users.noreply.github.com>
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.
data_processor. This class unified serializer anddeserializer interfaces to reduce code duplication. However, the
implementation is quite complex and enforces virtual dispatch for all
derived types.
write_inspectorandread_inspectorutility classes that injectan
operator(). The previous recursive argument unrolling in thedata_processorno longer has its place in C++17. The two new classesdispatch on
applymember functions of their subtype via CRTP and useif constexprto dispatch to the correct functions.serializeranddeserializerclasses. These two classesstill serve as base types for abstract inspectors, but no longer are
connected through a common base.
binary_serializerandbinary_deserializerclasses. These twoclasses are the workhorses in CAF that run in performance-critical
code. Hence, these implementations must reduce overhead as much as
possible. Because of this, they do not inherit from the abstract
interfaces and internally use the new
error_codeclass over thegeneric
errorclass.binary_serializerandbinary_deserializerto allCAF classes in addition to the generic versions.
Of course, the driving factor for these changes is performance. Here's the result of our
microbenchmark for the binary serialization for current CAFmaster:And heere's the result for this branch:
That's a speedup of 2.6 for
BinarySerializerand a speedup of 1.33 forBinaryDeserializer. Since especially the deserialization is the critical path, this should already make a notable difference. Our planned changes formessageand the way we handle RTTI should speed things up even more.This PR also removes some code related to
messagethat I didn't bother to update to the new API. Simply because we're going to remove them anyway.