quantum-core-x icon indicating copy to clipboard operation
quantum-core-x copied to clipboard

Refactor packet generator

Open MeikelLP opened this issue 1 year ago • 2 comments

Relates to #11

The previous implementation used C# 10's static interface members which where very handy. Sadly they came with a limitation: You can no longer use that interface as type parameter.

This has several disadvantages:

  • The type always needs to be boxed no matter if it's a value or reference type
  • Some weird constructs with delegates were required because we cannot use simple reflection to invoke anything related to those types

The new implementation works like this:

  • Static members from the interface where removed. You now require an instance to get static information like header, fixed packet size or similar. This sounds bad / annoying on the first sound. Also you might think this will cause a lot of unnecessary allocations but these packets can now be easily be pooled.
  • PacketInfo was simplified to no longer query data itself but require it to be inserted from outside (now only a data holder - no logic included)
  • PacketDeserializerGenerator and PacketSerializerGenerator opened up some APIs to be static internal which allows us to test them easily (Tests were added)

Overall this PR improves testability and maintainability as well as performance.

1. Attempt

First attempt was to implement packets as readonly ref structs because they are allocated on the stack by default, are immutable and cannot be passed / boxed outside of scope. Sadly this does not really work because we need to do some magic to invoke the packet handlers and pass down the packets. Activator or reflection in general cannot create ref structs and thus it's impossible to use a generic approach.

Another way would be to use C# Source Generators but this implies the limit to have source access to the packets (no plugin in packets possible). Additionally a "packet handler manger" would need to be generated. One for each assembly. This is not really working out.

2. Attempt

Using heavy array and object pooling (powered by Microsoft.Extensions.ObjectPool most commonly used in ASP.NET Core) I was able to significantly improve performance per packet read. It still uses (cached) reflection to invoke handlers but they are very fast. Packets are reused and recycled per invoke.

Here's a quick benchmark (1 packet read) of the packet readers:

Method Mean Error StdDev Ratio Gen0 Allocated Alloc Ratio
Reader 13,475.7 ns 234.16 ns 195.54 ns 1.00 0.1373 1224 B 1.00
Reader2 367.2 ns 6.19 ns 7.83 ns 0.03 0.0114 96 B 0.08

The benchmark can be seen in PacketReaderBenchmark.cs

Afterword

In the future the generator still requires some improvements when it comes to diagnostics and validation of user types.

MeikelLP avatar Apr 17 '24 20:04 MeikelLP

Quality Gate Failed Quality Gate failed

Failed conditions
D Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

sonarqubecloud[bot] avatar May 26 '24 17:05 sonarqubecloud[bot]