Provide `[ReadOnly]Span<char>` overloads for the methods

[Rekkonnect]

Feature Request

Details

Problem Description

The overloads currently exposed in the public API are all using string parameters. Since .NET Core, spans have been widely preferred for a variety of reasons involving performance, and are adopted in many cases.

Proposed Solution

Either change all string parameters to ReadOnlySpan<char> parameters, or provide overloads taking ROS<char> parameters, optionally using the OverloadResolutionPriority attribute to enhance their visibility.

For HashPassword, we can also provide overloads accepting the input as ROS<char>, and outputting the value into a user-provided Span<char> buffer:

public void HashPassword(ReadOnlySpan<char> inputKey, Span<char> output);
public void HashPassword(ReadOnlySpan<char> inputKey, ReadOnlySpan<char> salt, Span<char> output);
...

Alternatives considered

The current workaround is to manually convert the ROS<char> into a newly allocated string in order to pass it into this library's functions. This is suboptimal for critical performance scenaria.

Additional context

With the 5.0.0 release nearing, this would also be a good breaking change to include. Another release with even more breaking changes would probably not be ideal.

[ChrisMcKee]

@Rekkonnect its a work in progress as it kicked off splitting the framework from the standard a tad more brutally than the inline changes made the previous time in order to chase the chain through the various methods and to rebuild various benchmarks around the changes.

https://github.com/BcryptNet/bcrypt.net/tree/ReadonlySpanSupport

BenchmarkDotNet v0.14.0, Windows 11 (10.0.26100.3476)
AMD Ryzen 9 9900X, 1 CPU, 24 logical and 12 physical cores
.NET SDK 9.0.202
  [Host]     : .NET 9.0.3 (9.0.325.11113), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI
  Job-JCLYIQ : .NET 9.0.3 (9.0.325.11113), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI

Runtime=.NET 9.0  

Method	Mean	Error	StdDev	Ratio	RatioSD	Rank	Gen0	Allocated	Alloc Ratio
OldMethod	1.326 μs	0.0190 μs	0.0158 μs	1.00	0.02	2	0.0172	1008 B	1.00
NewMethod	1.023 μs	0.0168 μs	0.0149 μs	0.77	0.01	1	-	72 B	0.07

Method	Mean	Error	StdDev	Ratio	RatioSD	Rank	Gen0	Gen1	Gen2	Allocated	Alloc Ratio
EncodeBase64Unsized	36.29 ns	0.729 ns	0.682 ns	1.00	0.03	3	0.0049	-	-	280 B	1.00
EncodeBase64Sized	36.49 ns	0.742 ns	1.065 ns	1.01	0.03	3	0.0049	-	-	280 B	1.00
EncodeBase64AsBytes	15.95 ns	0.332 ns	0.465 ns	0.44	0.02	1	0.0013	-	-	72 B	0.26
EncodeBase64StackAlloc	22.56 ns	0.229 ns	0.214 ns	0.62	0.01	2	0.0016	-	-	72 B	0.26
EncodeBase64HeapSpanAlloc	16.07 ns	0.230 ns	0.215 ns	0.44	0.01	1	0.0017	0.0000	0.0000	-	0.00

Method	Mean	Error	StdDev	Median	Ratio	RatioSD	Rank	Gen0	Allocated	Alloc Ratio
OldMethod	1.363 μs	0.0236 μs	0.0197 μs	1.369 μs	1.00	0.02	2	0.0172	1008 B	1.00
NewMethod	1.111 μs	0.0219 μs	0.0395 μs	1.128 μs	0.82	0.03	1	-	-	0.00

Method	key	Mean	Error	StdDev	Ratio	RatioSD	Rank	Allocated	Alloc Ratio
TestHashValidateEnhanced	~!@#$(...)NBFRD [34]	167.3 ms	0.75 ms	0.66 ms	1.00	0.01	1	262.51 KB	1.00
TestHashValidateEnhancedv305Perf1	~!@#$(...)NBFRD [34]	167.7 ms	0.21 ms	0.17 ms	1.00	0.00	1	261.83 KB	1.00
TestHashValidateEnhancedv4Perf	~!@#$(...)NBFRD [34]	166.5 ms	3.29 ms	3.23 ms	0.99	0.02	1	261.85 KB	1.00
TestHashValidateEnhancedCurrent	~!@#$(...)NBFRD [34]	166.9 ms	3.06 ms	2.56 ms	1.00	0.02	1	5.51 KB	0.02
TestHashValidateEnhancedNet8Plus	~!@#$(...)NBFRD [34]	167.6 ms	0.22 ms	0.18 ms	1.00	0.00	1	4.81 KB	0.02

Method	key	salt	Mean	Error	StdDev	Ratio	RatioSD	Rank	Allocated	Alloc Ratio
TestHashValidateEnhanced	~!@#$(...)NBFRD [34]	$2a$1(...)rOvHe [29]	41.95 ms	0.121 ms	0.113 ms	1.00	0.00	1	70.29 KB	1.00
TestHashValidateEnhancedv3Perf	~!@#$(...)NBFRD [34]	$2a$1(...)rOvHe [29]	41.60 ms	0.797 ms	0.783 ms	0.99	0.02	1	69.62 KB	0.99
TestHashValidateEnhancedv4Perf	~!@#$(...)NBFRD [34]	$2a$1(...)rOvHe [29]	41.71 ms	0.794 ms	0.663 ms	0.99	0.02	1	69.62 KB	0.99
TestHashValidateEnhancedCurrent	~!@#$(...)NBFRD [34]	$2a$1(...)rOvHe [29]	41.73 ms	0.690 ms	0.611 ms	0.99	0.01	1	5.29 KB	0.08
TestHashValidateEnhancedNet8Plus	~!@#$(...)NBFRD [34]	$2a$1(...)rOvHe [29]	41.98 ms	0.062 ms	0.058 ms	1.00	0.00	1	4.63 KB	0.07

[Rekkonnect]

That's some very good improvement. I'm only not sure about returning a new byte[], we could have a parameter accepting a writable Span to contain the output bytes in. This would be passed by the user and should always accommodate the expected output byte count. That way we avoid one more allocation and it should also be faster.

The purpose of this is to allow passing a reusable output buffer across multiple calls of the function. An overload that allocates the returned byte array is also good to have because users may not want to reuse the result buffer, avoiding the necessary step of allocating the output buffer in calling the API.

[ChrisMcKee]

@Rekkonnect The base https://github.com/BcryptNet/bcrypt.net/blob/ReadonlySpanSupport/src/BCrypt.Net/BCryptBase.cs#L297 takes an ouput buffer. I've tried to keep the api relatively low effort, hence the overload can just return string as normal.

    internal static string CreatePasswordHash(ReadOnlySpan<char> inputKey, ReadOnlySpan<char> salt, HashType hashType = HashType.None, EnhancedHashDelegate enhancedHashKeyGen = null)
    {
        Span<char> outputBuffer = stackalloc char[60];
        CreatePasswordHash(inputKey, salt, outputBuffer, out var outputBufferWritten, hashType, enhancedHashKeyGen);
        return new string(outputBuffer[..outputBufferWritten]);
    }

and

        public static string HashPassword(ReadOnlySpan<char> inputKey, ReadOnlySpan<char> salt)
        {
            Span<char> outputBuffer = stackalloc char[60];
            HashPassword(inputKey, salt, outputBuffer, out var outputBufferWritten);
            return new string(outputBuffer[..outputBufferWritten]);
        }

There'll always be an allocation at some point with a string as the benchmarks are testing the same default method signature end to end so they're technically going to be worst case.

[Rekkonnect]

Right I missed that. It's all good then

[danielcrenna]

There are other reasons to use Span<T>, such as compatibility with CryptographicOperations.ZeroMemory and CryptographicOperations.FixedTimeEquals.

Is there a timeline for this?

[ChrisMcKee]

Soon. I got slightly distracted with benchmark.net and loading different package versions in.