akamsteeg / AtleX.HaveIBeenPwned

A fully async .NET Standard client library for the API of HaveIBeenPwned.com
https://www.nuget.org/packages/AtleX.HaveIBeenPwned/
MIT License
5 stars 0 forks source link

Replace double conversion for breached domain users #94

Closed akamsteeg closed 7 months ago

akamsteeg commented 7 months ago

Currently, when retrieving the breached users of a domain there's a double conversion from the JSON to the IEnumerable of DomainUser:

https://github.com/akamsteeg/AtleX.HaveIBeenPwned/blob/bfa57c78c057215f45f89f0f418913c75395b9f9/src/AtleX.HaveIBeenPwned/HaveIBeenPwnedClient.cs#L290C3-L322C3

First, we use the deserializer to convert the JSON to a Dictionary<string, IEnumerable<string>>. Then we go through that collection and create a new IEnumerable of DomainUser. That's quite a lot of code and bit wasteful.

Suggestion: Replace this two-stage approach with a custom JsonConvertor<IEnumerable<DomainUser>> that we register in the JsonSerializerOptions. This is supported for all supported TFMs so we don't need to introduce any new preprocessor directives.

akamsteeg commented 7 months ago

Promising results:

Old:

Method Toolchain Mean Ratio Allocated Alloc Ratio
GetBreachedDomainUsersAsync .NET 6.0 2.035 μs 1.36 3.39 KB 1.12
GetBreachedDomainUsersAsync .NET 8.0 1.501 μs 1.00 3.02 KB 1.00
GetBreachedDomainUsersAsync .NET Framework 4.8.1 4.551 μs 3.03 4.98 KB 1.65

New:

Method Toolchain Mean Ratio Allocated Alloc Ratio
GetBreachedDomainUsersAsync .NET 6.0 1.855 μs 1.38 2.81 KB 1.17
GetBreachedDomainUsersAsync .NET 8.0 1.345 μs 1.00 2.41 KB 1.00
GetBreachedDomainUsersAsync .NET Framework 4.8.1 4.198 μs 3.12 4.4 KB 1.83