Updating ProtobufSerializer to automatically lookup the MessageParser #3032
Conversation
The existing implementation of ProtobufSerializer requires RegisterFileDescriptor to be called with the file descriptor of every protobuf message that will be serialized. This adds a significant amount of boilerplate to serializer protobuf messages automatically. To work around this, I've updated the fromBinary to lookup the MessageParser at runtime based on the type. This brings it inline with how the JVM Akka protobuf serializer works.
|
Have you benchmarked it? |
|
I have not benchmarked this specific change, but benchmarks of Activator.CreateInstance with the default constructor, which we are using in this case, show that the overhead is minimal, plus we are caching the lookup, so at that point we'll only pay the cost of getting the key from the concurrent dictionary. |
|
TryGetValue is lockfree for ConcurrentDictionary btw |
|
So to be clear. What this PR aims to fix is the following. Correct ? I would like to see a few snippets of code about how you would retrieve the serializer instance and register your own descriptors. |
|
Correct, the idea is that if a user wants to send IMessage over the wire, the remoting protobuf serializer would automatically pick this up without any additional work from the user. |
| throw new ArgumentException("Need a protobuf message class to be able to serialize bytes using protobuf"); | ||
| } | ||
| // MethodParser is not in the cache, look it up with reflection | ||
| IMessage msg = Activator.CreateInstance(type) as IMessage; |
|
Going to run a local ping pong once I pull this down. the NBench numbers were in the 20k range. |
|
Benchmark numbers while I was running ~8 instances of Visual Studio on my machine: It's right on with top numbers I recorded before 1.3.0. Good to go on the perf IMHO. |
|
@Aaronontheweb I'm not sure that we are talking about the same things. Because we did not have this |
|
@alexvaluyskiy I'm talking about the work done on the 1.3.0 branch, when I added the benchmark. |
The existing implementation of ProtobufSerializer requires RegisterFileDescriptor to be called with the file descriptor of every protobuf message that will be serialized. This adds a significant amount of boilerplate to serialize protobuf messages automatically. To work around this, I've updated the fromBinary method to lookup the MessageParser at runtime based on the type. This brings it inline with how the JVM Akka protobuf serializer works.