-
Notifications
You must be signed in to change notification settings - Fork 417
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds JsonWebToken Constructor that accepts Memory<char> #2458
Conversation
// empty payload for JWE's {encrypted tokens}. | ||
Payload = new JsonClaimSet(); | ||
|
||
if (dotIndex == encodedJsonSpanCopy.Length) // TODO: Should this be encodedJsonSpanCopy.Length - 1? Using encodedJsonSpanCopy.Length doesn't add up |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thinking is that the value of EncodedJsonSpanCopy.Length will fall outside the range of the string and EncodedJsonSpanCopy.Length - 1 would be more appropriate.
3e91f27
to
1a96d3e
Compare
59cfab2
to
abdd893
Compare
In |
You can use profiling to search for more optimization: https://devblogs.microsoft.com/visualstudio/a-look-back-visual-studios-profiling-tool-advancements-in-2023/ |
Yes I have noticed and working towards finding a way to reduce allocations using the profiler. |
@keegan-caruso found that ReadJWETokenTests is not an appropriate benchmark for assessing JWE creation performance. Claims are not populated during JsonWebToken creation since the token is encrypted; they are only populated during validation when the token is decrypted. ValidateJWEAsyncTests is a more suitable benchmarks that creates and validates a JWE. On running this benchmark, there is no performance concern. There is a .5% reduction in memory allocation. sruthi/JsonWebTokenSpan BenchmarkDotNet=v0.13.5, OS=Windows 11 (10.0.22631.3007), VM=Hyper-V
AMD EPYC 7763, 1 CPU, 16 logical and 8 physical cores
.NET SDK=8.0.200-preview.23624.5
[Host] : .NET 8.0.1 (8.0.123.58001), X64 RyuJIT AVX2
MediumRun : .NET 8.0.1 (8.0.123.58001), X64 RyuJIT AVX2
Job=MediumRun MaxAbsoluteError=10.0000 ms IterationCount=15
LaunchCount=4 WarmupCount=10
dev BenchmarkDotNet=v0.13.5, OS=Windows 11 (10.0.22631.3007), VM=Hyper-V
AMD EPYC 7763, 1 CPU, 16 logical and 8 physical cores
.NET SDK=8.0.200-preview.23624.5
[Host] : .NET 8.0.1 (8.0.123.58001), X64 RyuJIT AVX2
MediumRun : .NET 8.0.1 (8.0.123.58001), X64 RyuJIT AVX2
Job=MediumRun MaxAbsoluteError=10.0000 ms IterationCount=15
LaunchCount=4 WarmupCount=10
|
src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonWebToken.PayloadClaimSet.cs
Show resolved
Hide resolved
benchmark/Microsoft.IdentityModel.Benchmarks/ReadJWETokenTests.cs
Outdated
Show resolved
Hide resolved
benchmark/Microsoft.IdentityModel.Benchmarks/ReadJWSTokenTests.cs
Outdated
Show resolved
Hide resolved
test/Microsoft.IdentityModel.Tokens.Tests/Base64UrlEncodingTests.cs
Outdated
Show resolved
Hide resolved
test/Microsoft.IdentityModel.JsonWebTokens.Tests/JsonWebTokenTests.cs
Outdated
Show resolved
Hide resolved
…ests.cs Co-authored-by: Peter <34331512+pmaytak@users.noreply.github.com>
Co-authored-by: Peter <34331512+pmaytak@users.noreply.github.com>
…ts.cs Co-authored-by: Peter <34331512+pmaytak@users.noreply.github.com>
…he end of the span
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you paste final benchmark results here? |
Introduces a JsonWebToken Constructor that accepts ReadOnlyMemory representation of the encodedToken
Description
This PR
Performance Assessment on .NET 8:
When using the preview version and opting for the constructor that accepts a string for the encoded token, the generation of a JWS yields a 3.95% decrease in memory usage, a 4.52% reduction in Gen0, and a 4.61% decrease in execution time compared to the latest release. The performance of the constructor that accepts ReadOnlyMemory is comparable to the constructor accepting a string for the encoded token, both for JWS and JWE.
Preview branch:
Dev (main) branch:
Why?
The new constructor allows a customer to leverage spans for slicing the encoded token directly from the Authorization Header eliminating additional String.Split operations on their end and can further improve performance.