Skip to content

feat(C#): add C# implementation (WIP) #2004

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

Open
wants to merge 53 commits into
base: main
Choose a base branch
from

Conversation

Handsome-cong
Copy link

@Handsome-cong Handsome-cong commented Jan 13, 2025

What does this PR do?

This PR provides a C# implementation of fury

Related issues

#686

Does this PR introduce any user-facing change?

  • Does this PR introduce any public API change?
  • Does this PR introduce any binary protocol compatibility change?

Benchmark

Sorry, something went wrong.

Handsome-cong and others added 29 commits December 26, 2024 22:42

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Handsome-cong and others added 24 commits January 13, 2025 17:06
This reverts commit 884fbdd.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@chaokunyang
Copy link
Collaborator

Hi @Handsome-cong , fury has been renamed to fory in #2263 , could you rename all dirs/paths/classnames from fury to fory in this PR?

}

_hasWrittenHeaderFlag = writerRef.WriteUInt8((byte)flag);
if (!_hasWrittenMagicNumber)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (!_hasWrittenMagicNumber)
if (!_hasWrittenHeaderFlag)

if (value is null)
{
writtenFlag = RefFlag.Null;
WriteRefFlag(ref writerRef, writtenFlag);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we just write one byte for null, but write a varint for ref tracking


namespace Fury.Serialization.Meta;

internal sealed class ReferenceMetaSerializer
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we keep consistent name as java/python? For example, we could name it RefResolver. In this way, when having serialization error across languages, we can easily go to the implementation of another language just based on the classname in exception trace

@chaokunyang
Copy link
Collaborator

@Handsome-cong Could you add license header to all csharp source files? And could we add some tests in Fury.Testing? We need some e2e tests to ensure the serialization itself really work like in #2112

@Handsome-cong
Copy link
Author

@chaokunyang Thanks for the suggestion, I'll fix these later. It may take a few days. I've been busy lately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants