-
Notifications
You must be signed in to change notification settings - Fork 15.5k
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
add C# deterministic serialization #13160
Closed
fmg-lydonchandra
wants to merge
6
commits into
protocolbuffers:main
from
fmg-lydonchandra:feature/12881_cs_serialization_deterministic
Closed
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
1092b7d
add basic deterministic serialization
fmg-lydonchandra 13087fe
removed unused WriteDeterministicTo
fmg-lydonchandra 590c61d
PR Review feedback take #1
fmg-lydonchandra f2e37d4
Use StringComparer.Ordinal to sort string keys
fmg-lydonchandra 1e2e6e9
Updated xml docs as per suggestion
fmg-lydonchandra ab7e01b
add SecuritySafeCritical attribute for Net462 to fix failing tests
fmg-lydonchandra File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -160,6 +160,33 @@ public long Position | |
} | ||
} | ||
|
||
/// <summary> | ||
/// Configures serialization to be deterministic. | ||
/// </summary> | ||
/// <remarks> | ||
/// The deterministic serialization guarantees that for a given binary, equal messages (defined by the | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: "The deterministic serialization" => "Deterministic serialization" |
||
/// equals methods in protos) will always be serialized to the same bytes. This implies: | ||
/// <list type="bullet"> | ||
/// <item><description>Repeated serialization of a message will return the same bytes</description></item> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: add period |
||
/// <item><description>Different processes of the same binary (which may be executing on different machines) | ||
/// will serialize equal messages to the same bytes.</description></item> | ||
/// </list> | ||
/// Note the deterministic serialization is NOT canonical across languages; it is also unstable | ||
/// across different builds with schema changes due to unknown fields. Users who need canonical | ||
/// serialization, e.g. persistent storage in a canonical form, fingerprinting, etc, should define | ||
/// their own canonicalization specification and implement the serializer using reflection APIs | ||
/// rather than relying on this API. | ||
/// Once set, the serializer will: (Note this is an implementation detail and may subject to | ||
/// change in the future) | ||
/// <list type="bullet"> | ||
/// <item><description>Sort map entries by keys in lexicographical order or numerical order. Note: For string | ||
/// keys, the order is based on comparing the UTF-16 code unit value of each character in the strings. | ||
/// The order may be different from the deterministic serialization in other languages where | ||
/// maps are sorted on the lexicographical order of the UTF8 encoded keys.</description></item> | ||
/// </list> | ||
/// </remarks> | ||
public bool Deterministic { get; set; } | ||
|
||
#region Writing of values (not including tags) | ||
|
||
/// <summary> | ||
|
@@ -485,7 +512,7 @@ public void WriteRawTag(byte b1, byte b2, byte b3, byte b4, byte b5) | |
#endregion | ||
|
||
#region Underlying writing primitives | ||
|
||
/// <summary> | ||
/// Writes a 32 bit value as a varint. The fast route is taken when | ||
/// there's enough buffer space left to whizz through without checking | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps "Configures whether or not serialization is deterministic." (It's a property, but the current text sounds sounds like it's a method.)