Add Odin Serializer to the benchmark (round 2) #1248
Conversation
- Benchmark results should now be correct
- Fix the issue where an exception stopped the entire benchmark suite - The exception still occurs but outside of Odin's code; it is now logged
|
The numbers are back to being surprising in the opposite direction - not sure how the benchmark is really set up, but those numbers are considerably worse than I'd expect given our own benchmarks and performance/GC measurements. Regardless, the exception issue itself is resolved. To provide some clarity, the issue was that during Odin Serializer's first-time initialization, it scans all loaded assemblies for extension points such as custom formatters - in this case, it was checking for the presence of an attribute that Odin uses to indicate that it has emitted a given assembly to support an AOT build. However, checking for any attribute via Assembly.IsDefined can cause the runtime to resolve all attributes declared on that assembly, and it seems one of the loaded assemblies had an attribute on it that depended on the System.Runtime.InteropServices.PInvoke assembly, which could be located. Therefore, Odin checking all assemblies for its own attribute causes an error to occur for a totally unrelated attribute on a loaded assembly. Odin itself has no dependency on System.Runtime.InteropServices.PInvoke. |
|
A SerializationContext and DeserializationContext must be disposed after use, or it will retain its old state, in which case you will get strange results like this. The same context can be disposed multiple times, though. However, I doubt this is what is causing the bad performance we're seeing - the cache claim is very lightweight and should not contribute materially to the serialization or deserialization time. I would suggest the initial using cached context pattern, as that is the "standard" way of claiming and using a context for an individual serialization call. Odin Serializer on .NET Core is a very new thing and it was just "ported" - perhaps everything else was simply slow on the old Unity runtimes and there the serializer was being compared to antique, slow versions of all other libraries or was just optimized for the performance characteristics for the APIs of those specific runtimes, or something like that. To clarify my confusion, in our initial benchmarks Odin compared very well against the likes of protobuf-net, and Odin's binary format target has since become ~5-20x faster (depending on circumstance), so seeing these numbers was a bit of a shock after all that optimization work I put in :D Regardless, it looks like it is at least working now, so these numbers are something I will have to look into later. |
|
Thanks for merging! What do you want to do about the data on the chart in the README? |
I guess it's time to update Odin's benchmark suite and add MessagePack-CSharp to it so we can compare them on a level playing field, then? |
I'm working on that now. |
|
Yes, we're currently swamped by other tasks but we should get our original benchmarks updated when there's space for it and add libraries like MessagePack-CSharp to the list. I wouldn't be surprised if we've simply fallen behind the times, though, and everything else has gotten much faster while we looked away. It'll be a while, though - updating the serializer benchmarks is not really on our internal roadmap as a priority at the moment, so either I do it in my free time or it'll have to wait for now or until someone else deigns to make a PR for it. Odin Serializer's core design certainly isn't very suited for the kinds of crazy speeds MessagePack can achieve with the way it's written. For that, a total rewrite of our serializer would be needed. Reading the source here, I'm very impressed - really great job! :D Many things have been done that I've long wanted to do, like the AutomataDictionary for member string resolution to avoid the extra GC and dictionary lookup overhead and the use of "new" .NET stuff like Span and Memory that we just don't have access to yet in Odin given the range of Unity versions we support (we use a fixed byte[] buffer and dump it through a Stream in chunks, which these days... ouch!). Perhaps one day I'll have time to give our serializer the total revisit it deserves. |
Is your benchmark suite open source? |
|
No, it was something we put together back in the day in a pretty helter-skelter fashion and we don't even have the benchmarking code any more, as it was lost. It was a very simple, naive benchmark with some warmup cycles for each test and then calculating an average over a massive number of iterations. Certainly nothing as rigorous as BenchmarkDotNet, which is why I'm inclined to trust these newer numbers more than our old benchmarks. Honestly, the pedigree of our benchmarks have long bothered me (and they're very old as well), but there keeps being other stuff to do instead with our core Odin Inspector product that uses the serializer. The serializer has not really been a focus of ours for some time, as it's been working well enough for our purposes and hasn't been cause for issue, while we've been rushing to keep up with Unity's rapid pace on other fronts. Getting them updated would certainly be very welcome, but like I said, it's not really been at the top of our minds. |
Understood, thank you for the info. |
Here's a second attempt at this PR. The difference is:
Here are the specs of my machine:
These are the results of the entire benchmark suite on my machine, including Odin:
Here are the full CSV and log files:
Benchmark.ShortRun_AllSerializerBenchmark_BytesInOut-report.csv
Benchmark.ShortRun_AllSerializerBenchmark_BytesInOut-20210527-121852.log