Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Akka.Persistence: design System.Memory APIs for all Journal / SnapshotStore internals #6060

Open
Aaronontheweb opened this issue Aug 8, 2022 · 4 comments

Comments

@Aaronontheweb
Copy link
Member

Is your feature request related to a problem? Please describe.

In order for #3740 to have any real-world benefit, the Journal / SnapshotStore implementations that consume the output from serialization also need to support System.Memory constructs so we can avoid unnecessary copying. This will, unfortunately, depend on whether or not the underlying database drivers support System.Memory as well.

Describe the solution you'd like

Ideally, we should replace all byte[] parameter overloads with Memory<T> or ReadOnlySequence<T> etc. Needs a full spec.

@Aaronontheweb
Copy link
Member Author

This needs a real spec.

@to11mtm
Copy link
Member

to11mtm commented Sep 8, 2022

I will suggest in earnest that the CommunityToolkit's APIs be considered for parts of this;

  • In the case of managed buffers being allocated internally (e.x. remoting), the MemoryOwner<T> Implementation in general is both 'safer' for use (i.e. versus MemoryPool<T> it actually handles making sure what you get back is the right size,) and has performance opportunities vs IMemoryOwner (As it has a direct .Span property, eliminating virt calls)
  • class IBufferWriterStream<TWriter> : Stream and their MemoryStream provide a facades that shim IBufferWriter and either ReadOnlyStream<byte> or IMemoryOwner<byte> into a stream, which may make it easier for some bits to be adapted in stages.
  • They also have some BufferWriter implementations around Stream and ArrayPool.

In general it's a bit unsung of a library but provides a lot of simple utility; for example IIRC PDN uses it

@Aaronontheweb
Copy link
Member Author

So I've been looking into doing this using those APIs here: Aaronontheweb/SystemMemorySerializer#2

I have a few reasons for thinking that this won't be feasible so long as we allow third party serialization - it makes the APIs for end-users much more complicated to implement correctly than they are right now. I'm thinking we're going to need to move towards a source-generator'd approach for creating strongly typed serializers for message types at some point down the road in order to fully take advantage of this, but that's a substantial change beyond the scope of 1.5 I think.

@to11mtm
Copy link
Member

to11mtm commented Sep 9, 2022

So I've been looking into doing this using those APIs here: Aaronontheweb/SystemMemorySerializer#2

I have a few reasons for thinking that this won't be feasible so long as we allow third party serialization - it makes the APIs for end-users much more complicated to implement correctly than they are right now. I'm thinking we're going to need to move towards a source-generator'd approach for creating strongly typed serializers for message types at some point down the road in order to fully take advantage of this, but that's a substantial change beyond the scope of 1.5 I think.

It would be tricky to do. The biggest concern I would have is in/around whether a source generator could pick up all the right cases; I'm thinking scenarios where there's already some form of reflection/LCG used to make certain types. The other concern I'd have (at least in the case of protobuf but I'll circle back to that) is making sure that any SourceGenerator-produced recordtypes have Identifiers generated that are both 'deterministic' as well as 'non-colliding'

I'll throw out there that I'm still -very- very hard in the MessagePack-CSharp camp. Aside from it AFAIK still being a good bit faster than protobuf in most cases, the overall approach is more declarative than Proto (at the tradeoff of not having a 'schema' defined aside from in-class, but in general it handles versioning better than Hyperion) and has some possibly compelling extensibility around typecodes and LZ4 compression (which may or may not have value in remoting, bytes over pipe vs serialization/deserialization time.)

MessagePack-CSharp also supports [Union(int key, Type subType)]/[Union(int key, string assemblyQualifiedTypeName)] attribute similar to the 'polymorphic' System.Text.Json APIs, as well as a 'Typeless' mode that can be useful for transitioning over.

@Aaronontheweb Aaronontheweb modified the milestones: 1.5.0, 1.5.1 Mar 2, 2023
@Aaronontheweb Aaronontheweb modified the milestones: 1.5.1, 1.5.2 Mar 15, 2023
@Aaronontheweb Aaronontheweb modified the milestones: 1.5.2, 1.5.3 Apr 6, 2023
@Aaronontheweb Aaronontheweb modified the milestones: 1.5.3, 1.5.4, 1.5.5 Apr 20, 2023
@Aaronontheweb Aaronontheweb modified the milestones: 1.5.5, 1.5.6, 1.5.7 May 4, 2023
@Aaronontheweb Aaronontheweb modified the milestones: 1.5.7, 1.5.8 May 17, 2023
@Aaronontheweb Aaronontheweb modified the milestones: 1.5.8, 1.5.9 Jun 15, 2023
@Aaronontheweb Aaronontheweb added this to the 1.6.0 milestone Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants