-
Notifications
You must be signed in to change notification settings - Fork 494
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
Performance: Fixes Reduces AuthorizationHelper memory allocations #1712
Conversation
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.
Please follow the required format: "[Internal] Category: (Adds|Fixes|Refactors) Description"
Examples:
Diagnostics: Adds GetElapsedClientLatency to CosmosDiagnostics
PartitionKey: Fixes null reference when using default(PartitionKey)
[v4] Client Encryption: Refactors code to external project
[Internal] Query: Adds code generator for CosmosNumbers for easy additions in the future.
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.
Please follow the required format: "[Internal] Category: (Adds|Fixes|Refactors) Description"
Examples:
Diagnostics: Adds GetElapsedClientLatency to CosmosDiagnostics
PartitionKey: Fixes null reference when using default(PartitionKey)
[v4] Client Encryption: Refactors code to external project
[Internal] Query: Adds code generator for CosmosNumbers for easy additions in the future.
AUTH EXCLUDED
AUTH BEFORE PERF CHANGES
AUTH AFTER PERF CHANGES
|
NOTE: Runs are from dev machine, latenciess might be unreliable. Lets just focus on allocations and GC's Surprisingly allocations are more higher with the new implementation. Host contextBenchmarkDotNet=v0.12.0, OS=Windows 10.0.19041 |
LONG Job run details BenchmarkDotNet=v0.12.0, OS=Windows 10.0.19041 Job=LongRun IterationCount=100 LaunchCount=3 AUTH BEFORE PERF CHANGES
AUTH AFTER PERF CHANGES
|
...soft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Performance.Tests/Mocks/MockDocumentClient.cs
Outdated
Show resolved
Hide resolved
...soft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Performance.Tests/Mocks/MockDocumentClient.cs
Show resolved
Hide resolved
Hey Kiran - I am unable to reproduce the same difference - can we chat and figure out what the differences might be with what i'm seeing? |
...tests/Microsoft.Azure.Cosmos.Performance.Tests/Benchmarks/MasterKeyAuthorizationBenchmark.cs
Outdated
Show resolved
Hide resolved
...tests/Microsoft.Azure.Cosmos.Performance.Tests/Benchmarks/MasterKeyAuthorizationBenchmark.cs
Outdated
Show resolved
Hide resolved
Latest run:
|
...tests/Microsoft.Azure.Cosmos.Performance.Tests/Benchmarks/MasterKeyAuthorizationBenchmark.cs
Show resolved
Hide resolved
Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Performance.Tests/Query/EndToEnd.cs
Show resolved
Hide resolved
8d22dc7
to
28f5cc2
Compare
Closing due to in-activity, pease feel free to re-open. |
Including exclusive auth performance micro benchmark.
Changes includes:
After the changes
RPS: 2-3% UP
Latency: 3-6% UP
GEN0: 30% DOWN
Allocations: 28% DOWN
Before changes:
Branch with-out Auth changes & perf benchmark: users/kirankk/auth_baseline
NOTE: Please change the target to netcoreapp3.1
Also the latency is higher with the new optimization.
BenchmarkDotNet=v0.12.0, OS=Windows 10.0.19041
Intel Core i7-8650U CPU 1.90GHz (Kaby Lake R), 1 CPU, 8 logical and 4 physical cores
.NET Core SDK=3.1.302
[Host] : .NET Core 3.1.6 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.31603), X64 RyuJIT
DefaultJob : .NET Core 3.1.6 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.31603), X64 RyuJIT
BenchmarkDotNet=v0.12.0, OS=ubuntu 18.04
Intel Xeon Platinum 8171M CPU 2.60GHz, 1 CPU, 8 logical and 4 physical cores
.NET Core SDK=3.1.301
[Host] : .NET Core 3.1.5 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.27001), X64 RyuJIT
DefaultJob : .NET Core 3.1.5 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.27001), X64 RyuJIT