Skip to content

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

@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.

2 participants