search

DuendeSoftware / IdentityServer

The most flexible and standards-compliant OpenID Connect and OAuth 2.x framework for ASP.NET Core
https://duendesoftware.com/products/identityserver
Other
1.45k stars 337 forks source link

Use SHA*.HashData() one-shot methods #1573

Open martincostello opened 3 months ago

martincostello commented 3 months ago

Use SHA*.HashData() one-shot methods to avoid allocating SHA* objects.

This is something I just happened to notice while I was debugging an issue in something using IdentityServer in a sample and I stepped into the decompiled code.

The only usages of SHA*.Create() left after this change are these:

https://github.com/DuendeSoftware/IdentityServer/blob/e9860c6488f90e8fbc11a4452b9dd111dbfae933/src/IdentityServer/Configuration/CryptoHelper.cs#L84-L100

As this is a public method I left it as-is, but refactored the only caller within the codebase itself to use the one-shot methods and not depend on it anymore.

josephdecock commented 1 month ago

Thanks for the PR! Is this optimization addressing a performance problem you're seeing in practice? Regardless, this seems like a good thing to do, but merging it into main would mean it won't be available until 7.1 releases (probably December/January).

martincostello commented 1 month ago

Is this optimization addressing a performance problem you're seeing in practice?

No, just something I stumbled across when looking at the code.

josephdecock commented 1 month ago

Okay thanks. I want to think a bit more about GetHashAlgorithmForSigningAlgorithm (maybe we deprecate it), but this looks great! For now I'll leave this open as a reminder.