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

[API Proposal]: XxHash128 #77885

Closed
xoofx opened this issue Nov 4, 2022 · 6 comments · Fixed by #77944
Closed

[API Proposal]: XxHash128 #77885

xoofx opened this issue Nov 4, 2022 · 6 comments · Fixed by #77944
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.IO.Hashing
Milestone

Comments

@xoofx
Copy link
Member

xoofx commented Nov 4, 2022

Background and motivation

Followup of the previous API proposal for XX3 64 bit #75948

The current XxHash3 implementation provides a 64 bit hash. But XXH3 128 is also quite useful for e.g fast content adressable storage scenarios.

This proposal is about adding a XxHash128 API and implementation.

API Proposal

namespace System.IO.Hashing;

public sealed partial class XxHash128 : NonCryptographicHashAlgorithm
{
    public XxHash128();
    public XxHash128(long seed);

    public static byte[] Hash(byte[] source);
    public static byte[] Hash(byte[] source, long seed);
    public static byte[] Hash(ReadOnlySpan<byte> source, long seed = 0);
    public static int Hash(ReadOnlySpan<byte> source, Span<byte> destination, long seed = 0);
    public static bool TryHash(ReadOnlySpan<byte> source, Span<byte> destination, out int bytesWritten, long seed = 0);

    // .NET 7+
    public static UInt128 HashToIntU128(ReadOnlySpan<byte> source, long seed = 0);
    public UInt128 GetCurrentHashAsIntU128();
}

API Usage

int bytesWritten = XxHash128.Hash(source, destination);

Alternative Designs

No response

Risks

No response

@xoofx xoofx added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Nov 4, 2022
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Nov 4, 2022
@ghost
Copy link

ghost commented Nov 4, 2022

Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

Followup of the previous API proposal for XX3 64 bit #75948

The current XxHash3 implementation provides a 64 bit hash. But XXH3 128 is also quite useful for e.g fast content adressable storage scenarios.

This proposal is about adding a XxHash128 API and implementation.

API Proposal

namespace System.IO.Hashing;

public sealed partial class XxHash128 : NonCryptographicHashAlgorithm
{
    public XxHash128();
    public XxHash128(int seed);

    public static byte[] Hash(byte[] source);
    public static byte[] Hash(byte[] source, int seed);
    public static byte[] Hash(ReadOnlySpan<byte> source, int seed = 0);
    public static int Hash(ReadOnlySpan<byte> source, Span<byte> destination, int seed = 0);
    public static bool TryHash(ReadOnlySpan<byte> source, Span<byte> destination, out int bytesWritten, int seed = 0);
}

API Usage

int bytesWritten = XxHash128.Hash(source, destination);

Alternative Designs

No response

Risks

No response

Author: xoofx
Assignees: -
Labels:

api-suggestion, area-System.Security

Milestone: -

@stephentoub stephentoub added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation untriaged New issue has not been triaged by the area owner labels Nov 4, 2022
@stephentoub
Copy link
Member

Is it correct that the seeds are ints instead of longs?

@xoofx
Copy link
Member Author

xoofx commented Nov 4, 2022

Is it correct that the seeds are ints instead of longs?

Indeed, it should be long, I have updated.

@stephentoub
Copy link
Member

I've marked this as api-ready-for-review. I believe this is just a formality: we basically rubber-stamped this when reviewing XxHash3, saying we'd add XxHash128 if there was a demonstrated need, and the proposed API follows the same shape/pattern as all the other algorithms in the library.
cc: @bartonjs

@MihaZupan MihaZupan added this to the 8.0.0 milestone Nov 4, 2022
@MihaZupan
Copy link
Member

Moving this to 8.0 given that that is what XxHash3 was aimed at

@xoofx xoofx mentioned this issue Nov 5, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Nov 5, 2022
@terrajobst
Copy link
Member

terrajobst commented Nov 8, 2022

Video

namespace System.IO.Hashing;

public sealed partial class XxHash128 : NonCryptographicHashAlgorithm
{
    public XxHash128();
    public XxHash128(long seed);

    public static byte[] Hash(byte[] source);
    public static byte[] Hash(byte[] source, long seed);
    public static byte[] Hash(ReadOnlySpan<byte> source, long seed = 0);
    public static int Hash(ReadOnlySpan<byte> source, Span<byte> destination, long seed = 0);
    public static bool TryHash(ReadOnlySpan<byte> source, Span<byte> destination, out int bytesWritten, long seed = 0);
}

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Nov 8, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Dec 2, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jan 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.IO.Hashing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants