Skip to content
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

Regressions in System.Security.Cryptography.Tests.Perf_Rfc2898DeriveBytes #65719

Closed
performanceautofiler bot opened this issue Feb 22, 2022 · 6 comments
Closed
Labels
arch-x64 area-System.Security runtime-coreclr specific to the CoreCLR runtime untriaged New issue has not been triaged by the area owner

Comments

@performanceautofiler
Copy link

Run Information

Architecture x64
OS alpine 3.12
Baseline ae24a8f8dd745e24384f19c22f85b28aa1f33cd6
Compare 1615de6ccc1290be168c1de290429489c489efde
Diff Diff

Regressions in System.Security.Cryptography.Tests.Perf_Rfc2898DeriveBytes

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio Baseline ETL Compare ETL
DeriveBytes - Duration of single invocation 4.70 ms 5.72 ms 1.22 0.02 True

graph
Test Report

Repro

git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Security.Cryptography.Tests.Perf_Rfc2898DeriveBytes*'

Payloads

Baseline
Compare

Histogram

System.Security.Cryptography.Tests.Perf_Rfc2898DeriveBytes.DeriveBytes


Description of detection logic

IsRegressionChecked: Marked as regression because the three check build points were 0.05 greater than the baseline.
IsImprovementBase: Marked as not an improvement because the compare was not 5% less than the baseline, or the value was too small.
IsRegressionBase: Marked as regression because the compare was 5% greater than the baseline, and the value was not too small.
IsRegressionChecked: Marked as regression because the three check build points were 0.05 greater than the baseline.
IsRegressionWindowed: Marked as regression because 5.717147306089743 > 4.941921660661057.
IsChangePoint: Marked as a change because one of 2/14/2022 7:55:17 PM, 2/22/2022 12:12:47 AM falls between 2/13/2022 12:11:49 PM and 2/22/2022 12:12:47 AM.
IsRegressionStdDev: Marked as regression because -51.537669114894946 (T) = (0 -5621127.050861655) / Math.Sqrt((575541557.8977764 / (22)) + (13139154971.515217 / (45))) is less than -1.997137908391408 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (22) + (45) - 2, .025) and -0.1955079817183199 = (4701873.293043458 - 5621127.050861655) / 4701873.293043458 is less than -0.05.
IsImprovementBase: Marked as not an improvement because the compare was not 5% less than the baseline, or the value was too small.
IsChangeEdgeDetector: Marked as regression because Edge Detector said so.

Docs

Profiling workflow for dotnet/runtime repository
Benchmarking workflow for dotnet/runtime repository

@performanceautofiler performanceautofiler bot added alpine 3.12 untriaged New issue has not been triaged by the area owner labels Feb 22, 2022
@kunalspathak kunalspathak changed the title [Perf] Changes at 2/15/2022 12:24:52 AM Regressions in System.Security.Cryptography.Tests.Perf_Rfc2898DeriveBytes Feb 22, 2022
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@kunalspathak kunalspathak transferred this issue from dotnet/perf-autofiling-issues Feb 22, 2022
@kunalspathak
Copy link
Member

@ghost
Copy link

ghost commented Feb 22, 2022

Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

Run Information

Architecture x64
OS alpine 3.12
Baseline ae24a8f8dd745e24384f19c22f85b28aa1f33cd6
Compare 1615de6ccc1290be168c1de290429489c489efde
Diff Diff

Regressions in System.Security.Cryptography.Tests.Perf_Rfc2898DeriveBytes

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio Baseline ETL Compare ETL
DeriveBytes - Duration of single invocation 4.70 ms 5.72 ms 1.22 0.02 True

graph
Test Report

Repro

git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Security.Cryptography.Tests.Perf_Rfc2898DeriveBytes*'

Payloads

Baseline
Compare

Histogram

System.Security.Cryptography.Tests.Perf_Rfc2898DeriveBytes.DeriveBytes


Description of detection logic

IsRegressionChecked: Marked as regression because the three check build points were 0.05 greater than the baseline.
IsImprovementBase: Marked as not an improvement because the compare was not 5% less than the baseline, or the value was too small.
IsRegressionBase: Marked as regression because the compare was 5% greater than the baseline, and the value was not too small.
IsRegressionChecked: Marked as regression because the three check build points were 0.05 greater than the baseline.
IsRegressionWindowed: Marked as regression because 5.717147306089743 > 4.941921660661057.
IsChangePoint: Marked as a change because one of 2/14/2022 7:55:17 PM, 2/22/2022 12:12:47 AM falls between 2/13/2022 12:11:49 PM and 2/22/2022 12:12:47 AM.
IsRegressionStdDev: Marked as regression because -51.537669114894946 (T) = (0 -5621127.050861655) / Math.Sqrt((575541557.8977764 / (22)) + (13139154971.515217 / (45))) is less than -1.997137908391408 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (22) + (45) - 2, .025) and -0.1955079817183199 = (4701873.293043458 - 5621127.050861655) / 4701873.293043458 is less than -0.05.
IsImprovementBase: Marked as not an improvement because the compare was not 5% less than the baseline, or the value was too small.
IsChangeEdgeDetector: Marked as regression because Edge Detector said so.

Docs

Profiling workflow for dotnet/runtime repository
Benchmarking workflow for dotnet/runtime repository

Author: performanceautofiler[bot]
Assignees: -
Labels:

area-System.Security, untriaged, refs/heads/main, x64, Regression, RunKind=micro, CoreClr, alpine 3.12

Milestone: -

@vcsjones
Copy link
Member

Well, that's 10,000 HMAC invocations.

https://github.com/dotnet/performance/blob/d7dac8a7ca12a28d099192f8a901cf8e30361384/src/benchmarks/micro/libraries/System.Security.Cryptography/Perf.Rfc2898DeriveBytes.cs#L13

So 20,000 additional calls to ERR_clear_error. Each iteration does CryptoNative_HmacUpdate and CryptoNative_HmacFinal. If it regressed by about 1ms, then that means each invocation of ERR_clear_error added about 50ns of overhead, assuming that is the culprit. @bartonjs does that line up with your expectations?

@bartonjs
Copy link
Member

Yeah. I'm not really worried about that differential in isolation.

According to
https://pvscmdupload.blob.core.windows.net/reports/allTestHistory%2frefs%2fheads%2fmain_x64_ubuntu%2018.04%2fSystem.Security.Cryptography.Tests.Perf_Rfc2898DeriveBytes.DeriveBytes.html vs
https://pvscmdupload.blob.core.windows.net/reports/allTestHistory%2frefs%2fheads%2fmain_x64_Windows%2010.0.18362%2fSystem.Security.Cryptography.Tests.Perf_Rfc2898DeriveBytes.DeriveBytes.html

the operation is still faster on Linux than Windows (with the same hardware).

The scale of the perf degradation, combined with PBKDF2 itself not being used in a loop (except by password breakers) makes me feel like this is an acceptable consequence of keeping the error queue better managed.

@vcsjones
Copy link
Member

That makes sense. The one-shot is probably more interesting to measure from a performance perspective.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-x64 area-System.Security runtime-coreclr specific to the CoreCLR runtime untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

6 participants