[API Proposal]: Consider adding a Clone() Method to the hash implementations

[markusschaber]

Background and motivation

Our (rather old) home-grown CRC32 implementation has a Clone() method, which clones the internal state of the CRC32 into a new instance, which is then usable independently of the original instance.

It is used in several places in our existing Software, for rather weird backwards compatibility reasons with existing protocols and file formats, so in those cases, we cannot migrate to the System.IO.Hashing package and benefit from the performance optimizations (ARM intrinsics, Vectorization etc.).

Implementing Clone should be easy and straight forward (just create a new instance and copy the state over). Below, I'll specify it for CRC32, but it can be easily expanded to all algorithms.

If you're interested, I could try to work out a fully-fledged pull request.

API Proposal

namespace System.IO.Hashing;

public sealed partial class Crc32
{
    /// <summary> Returns a clone of the current instance, with a copy of the internal state. </summary>
    public Crc32 Clone();
}

public sealed partial class Crc64
{
    public Crc64 Clone();
}

public sealed partial class XxHash3
{
    public XxHash3 Clone();
}

public sealed partial class XxHash32
{
    public XxHash32 Clone();
}

public sealed partial class XxHash64
{
    public XxHash64 Clone();
}

public sealed partial class XxHash128
{
    public XxHash128 Clone();
}

API Usage

// Fancy the value
var crc = new Crc32();
crc.Append(SomeCommonData);

var clone = crc.Clone();

crc.Append(SomeDataVariantA);
var aSum = crc.GetCurrentHashAsUint32();

clone.Append(SomeDataVariantB);
var bSum = clone.GetCurrentHashAsUnit32();

Alternative Designs

We could implement the ICloneable interface. The advantage is that we implement a standard interface. The disadvantage is that ICloneable.Clone() returns object, requiring an additional cast before the cloned instance can be used. To remedy this, the ICloneable interface could be implemented explicitly, in addition to the typed Clone() method shown above, so we can have the best of both worlds. But to be honest, I don't see a real use case for implementing this interface, and it can always be added later as an explicit interface implementation without breaking existing users.

We could use a copy constructor like var clone = new Crc32(existingInstance); or a public static Crc32 Clone(Crc32 instance) method. However, I prefer the Clone() method, which is an established pattern to copy instances.

Risks

I don't see any risks of breaking changes, performance regressions etc. (apart from the slight increase in code size).

[dotnet-policy-service]

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

[bartonjs]

We could implement the ICloneable interface.

IClonable will never be used again in this repository. Instead we do the clone-convention, which is what you proposed 😄.

The proposal seems consistent with the cryptographic IncrementalHash, so I've gone ahead and expanded it to all NC-Hash types and marked it ready for review.

[hez2010]

IClonable will never be used again in this repository. Instead we do the clone-convention, which is what you proposed 😄.

Alternatively we can introduce a new interface IClonable<T>.

[vcsjones]

Alternatively we can introduce a new interface IClonable<T>.

It has been proposed a number of times: #77121, #77077, #63707, #37561, etc.

[markusschaber]

I think we shouldn't try to solve the ICloneable<T> discussion within this proposal. As there's no accepted generic version, and the non-generic version is obsolete, we should implement neither.

[bartonjs]

Video

• Looks good as proposed

namespace System.IO.Hashing;

public sealed partial class Crc32
{
    /// <summary> Returns a clone of the current instance, with a copy of the internal state. </summary>
    public Crc32 Clone();
}

public sealed partial class Crc64
{
    public Crc64 Clone();
}

public sealed partial class XxHash3
{
    public XxHash3 Clone();
}

public sealed partial class XxHash32
{
    public XxHash32 Clone();
}

public sealed partial class XxHash64
{
    public XxHash64 Clone();
}

public sealed partial class XxHash128
{
    public XxHash128 Clone();
}

[vcsjones]

@markusschaber

If you're interested, I could try to work out a fully-fledged pull request.

The API was approved. Were you still interested in submitting a pull request?