search

andrewlock / PwnedPasswords

An ASP.NET Core Identity validator that checks for PwnedPasswords
MIT License
103 stars 12 forks source link

Investigate whether we can decrease the memory/string allocation overhead #15

Closed SeanFarrow closed 5 years ago

SeanFarrow commented 5 years ago

Given we are slicing strings in various places, it's worth investigating whether Span will make a difference in terms of performance.

We may not see performance gains when checking a single password, but these may be evident when using the password client implementations in a local api.

andrewlock commented 5 years ago

To be clear, Span will almost certainly make some difference in terms of performance, but whether it's worth adding the extra dependency plus less readable code is another question. Also, by it's nature the public API for the lib will need to take in a string (as that's what the Identity API surface defines), so it may be worth limiting Span use to only individual implementations under the hood. Then again, if we're using it somewhere, why not everywhere🤷‍♂️ See how it goes!

SeanFarrow commented 5 years ago

Ok, as per: this issue, we can't use Span in methods that are async.

I still think we'll get some benefit, we may need to move some of the internals around though.

SeanFarrow commented 5 years ago

Closing as the API's are not available on .Net standard 2.0 to decrease performance in the way I would have liked.

I will consider reopening this when the next LTS version of .Net Core is released (probably .Net Core 3.1) as we will have .Net Standard 2.1 support which will give us the API's we need to make Span worthwhile.