server
server copied to clipboard
[PS-1471] Create Allocation Free `EncryptedStringAttribute` validation
Type of change
- [ ] Bug fix
- [ ] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [x] Other
Objective
We validate the shape of incoming encrypted strings but we don't actually do anything with the data so every bit of data that is allocated for that adds to the heap that then has to be deallocated later.
| Method | Branch | EncryptedString | Mean | Error | StdDev | Gen0 | Gen1 | Allocated |
|---|---|---|---|---|---|---|---|---|
| IsValid | encrypted-string-perf | 2.Kl(...)uXg= [5616] | 375.8 ns | 4.47 ns | 3.49 ns | - | - | - |
| IsValid | encrypted-string-perf | 2.QmF(...)ydA== [52] | 137.8 ns | 2.79 ns | 2.87 ns | - | - | - |
| IsValid | master | 2.Kl(...)uXg= [5616] | 15,731.6 ns | 304.21 ns | 362.14 ns | 4.3640 | 0.2441 | 27425 B |
| IsValid | master | 2.QmF(...)ydA== [52] | 314.5 ns | 6.24 ns | 6.93 ns | 0.0839 | - | 528 B |
NOTE: It's worth stating that a this attribute is called a few times in each request that uses them. For login it's used probably 4 times (Name, Username, Password, Uri) so the savings would be 4x at least what is shown in low [52] character input.
Code changes
- bitwarden-server.sln: Added perf project so we can have a single place to add benchmarks.
-
perf/MicroBenchmarks/Core/EncryptedStringAttributeTests.cs: A benchmark that tests the current implementation of
IsValidwith two parameters, one that is a general "password" and one that is 5 paragraphs of lorem ipsum. -
src/Core/Utilities/EncryptedStringAttribute.cs: Refactor of
IsValidto be quicker and to have less allocations. -
src/Core/Utilities/SpanExtensions.cs: Add Span extension for splitting a ReadOnlySpan
by a certain character and returning whether it was successful and using outparameters to return both a span before and after the split character. -
test/Core.Test/Utilities/EncryptedStringAttributeTests.cs: Added tests that more thourougly test the possible inputs to the validation and it's expected input
- These were added before any changes to the underlying logic was made and all were successful against that with no uncovered branches (except for one unreachable branch).
Before you submit
- Please check for formatting errors (
dotnet format --verify-no-changes) (required) - If making database changes - make sure you also update Entity Framework queries and/or migrations
- Please add unit tests where it makes sense to do so (encouraged but not required)
- If this change requires a documentation update - notify the documentation team
- If this change has particular deployment requirements - notify the DevOps team