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.Buffers.Text.Tests.Utf8FormatterTests #78041

Closed
performanceautofiler bot opened this issue Nov 8, 2022 · 6 comments
Closed

Regressions in System.Buffers.Text.Tests.Utf8FormatterTests #78041

performanceautofiler bot opened this issue Nov 8, 2022 · 6 comments
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI tenet-performance Performance related issue tenet-performance-benchmarks Issue from performance benchmark
Milestone

Comments

@performanceautofiler
Copy link

Run Information

Architecture x64
OS ubuntu 18.04
Baseline ce041c81b689499cc02b0a4d6914f53d7c44b5c7
Compare 09c81d8bef22ee8350b86911b20715a19ffc05ab
Diff Diff

Regressions in System.Buffers.Text.Tests.Utf8FormatterTests

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio Baseline ETL Compare ETL
FormatterInt64 - Duration of single invocation 23.03 ns 29.21 ns 1.27 0.02 False
FormatterUInt32 - Duration of single invocation 9.91 ns 12.82 ns 1.29 0.01 False

graph
graph
Test Report

Repro

git clone https://github.com/dotnet/performance.git
python3 .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Buffers.Text.Tests.Utf8FormatterTests*'

Related Issues

Regressions

Improvements

Payloads

Baseline
Compare

Histogram

Edge Detector Info

Collection Data

System.Buffers.Text.Tests.Utf8FormatterTests.FormatterInt64(value: 9223372036854775807)


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.
IsRegressionWindowed: Marked as regression because 29.212626133931302 > 24.328699978006973.
IsChangePoint: Marked as a change because one of 9/3/2022 11:44:47 AM, 10/11/2022 2:16:14 AM, 11/4/2022 9:06:14 PM, 11/8/2022 2:29:58 AM falls between 10/30/2022 12:44:54 AM and 11/8/2022 2:29:58 AM.
IsRegressionStdDev: Marked as regression because -75.5984167211087 (T) = (0 -29.211252705184588) / Math.Sqrt((0.09798937478021806 / (41)) + (0.032984641590120704 / (9))) is less than -2.010634757623041 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (41) + (9) - 2, .025) and -0.25216090685751974 = (23.32867329207273 - 29.211252705184588) / 23.32867329207273 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 not as a regression because Edge Detector said so.
IsChangeEdgeDetector: Marked not as a regression because Edge Detector said so.

```#### System.Buffers.Text.Tests.Utf8FormatterTests.FormatterUInt32(value: 4294967295)

```log

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.
IsRegressionWindowed: Marked as regression because 12.823637156237847 > 10.427979517056931.
IsChangePoint: Marked as a change because one of 9/3/2022 11:44:47 AM, 10/11/2022 2:16:14 AM, 11/4/2022 9:06:14 PM, 11/8/2022 2:29:58 AM falls between 10/30/2022 12:44:54 AM and 11/8/2022 2:29:58 AM.
IsRegressionStdDev: Marked as regression because -76.51693901950912 (T) = (0 -13.032252353193137) / Math.Sqrt((0.0016959854340174783 / (41)) + (0.014481213515299272 / (9))) is less than -2.010634757623041 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (41) + (9) - 2, .025) and -0.3132382797505535 = (9.92375302650223 - 13.032252353193137) / 9.92375302650223 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 not as a regression because Edge Detector said so.
IsChangeEdgeDetector: Marked not as a regression because Edge Detector said so.

Docs

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

@performanceautofiler performanceautofiler bot added CoreClr untriaged New issue has not been triaged by the area owner labels Nov 8, 2022
@EgorBo EgorBo changed the title [Perf] Linux/x64: 2 Regressions on 11/5/2022 3:01:26 AM Regressions in System.Buffers.Text.Tests.Utf8FormatterTests Nov 8, 2022
@EgorBo EgorBo removed refs/heads/main untriaged New issue has not been triaged by the area owner labels Nov 8, 2022
@EgorBo EgorBo transferred this issue from dotnet/perf-autofiling-issues Nov 8, 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.

@EgorBo
Copy link
Member

EgorBo commented Nov 8, 2022

Likely regressed from #77137 (e.g. changed alignment?) cc @TIHan

@EgorBo EgorBo added tenet-performance Performance related issue tenet-performance-benchmarks Issue from performance benchmark area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Nov 8, 2022
@JulieLeeMSFT JulieLeeMSFT added this to the 8.0.0 milestone Dec 27, 2022
@JulieLeeMSFT
Copy link
Member

Ping @TIHan .

@TIHan
Copy link
Contributor

TIHan commented Dec 28, 2022

It looks like the optimization I did with x << 3 and x << 2 may have caused it. Specifically looking at the diffs of the FormatterInt64 test:
image

Now, I manually tested, on my machine, FormatterInt64 with the correct input value, 9223372036854775807 showing before and after regarding #77137. It either showed an improvement or no difference; so it may be machine specific? Maybe newer hardware it is an improvement? The results below:

Benchmark 1

    public class Utf8FormatterTests
    {
        private readonly byte[] _destination = new byte[1000];

        // This is using TryFormat(long value...).
        [Benchmark]
        public bool FormatterInt64() => Utf8Formatter.TryFormat(9223372036854775807, _destination, out _);
    }

Before #77137

BenchmarkDotNet=v0.13.3, OS=Windows 11 (10.0.22621.963)
AMD Ryzen 9 7950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK=7.0.101
  [Host]     : .NET 7.0.1 (7.0.122.56804), X64 RyuJIT AVX2
  Job-TYOOUA : .NET 8.0.0 (42.42.42.42424), X64 RyuJIT AVX2

Toolchain=CoreRun

|         Method |     Mean |    Error |   StdDev |
|--------------- |---------:|---------:|---------:|
| FormatterInt64 | 19.25 ns | 0.033 ns | 0.030 ns |

After #77137

BenchmarkDotNet=v0.13.3, OS=Windows 11 (10.0.22621.963)
AMD Ryzen 9 7950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK=7.0.101
  [Host]     : .NET 7.0.1 (7.0.122.56804), X64 RyuJIT AVX2
  Job-OYHQOX : .NET 8.0.0 (42.42.42.42424), X64 RyuJIT AVX2

Toolchain=CoreRun

|         Method |     Mean |    Error |   StdDev |
|--------------- |---------:|---------:|---------:|
| FormatterInt64 | 18.35 ns | 0.092 ns | 0.086 ns |

The example above shows that #77137 is improvement, at least on my machine.

Benchmark 2

    public class Utf8FormatterTests
    {
        private readonly byte[] _destination = new byte[1000];

        [MethodImpl(MethodImplOptions.NoInlining)]
        public bool FormatterInt64(long value) => Utf8Formatter.TryFormat(value, _destination, out _);

        [Benchmark]
        public bool FormatterInt64() => FormatterInt64(9223372036854775807);
    }

Before #77137

BenchmarkDotNet=v0.13.3, OS=Windows 11 (10.0.22621.963)
AMD Ryzen 9 7950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK=7.0.101
  [Host]     : .NET 7.0.1 (7.0.122.56804), X64 RyuJIT AVX2
  Job-VVQGGN : .NET 8.0.0 (42.42.42.42424), X64 RyuJIT AVX2

Toolchain=CoreRun

|         Method |     Mean |    Error |   StdDev |
|--------------- |---------:|---------:|---------:|
| FormatterInt64 | 19.61 ns | 0.080 ns | 0.075 ns |

After #77137

BenchmarkDotNet=v0.13.3, OS=Windows 11 (10.0.22621.963)
AMD Ryzen 9 7950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK=7.0.101
  [Host]     : .NET 7.0.1 (7.0.122.56804), X64 RyuJIT AVX2
  Job-GQAVBD : .NET 8.0.0 (42.42.42.42424), X64 RyuJIT AVX2

Toolchain=CoreRun

|         Method |     Mean |    Error |   StdDev |
|--------------- |---------:|---------:|---------:|
| FormatterInt64 | 19.62 ns | 0.047 ns | 0.039 ns |

The example above shows that there is basically no difference.


@dotnet/jit-contrib how should I move forward with this? Maybe test more hardware?

@TIHan
Copy link
Contributor

TIHan commented Jan 4, 2023

cc @EgorBo

@EgorBo
Copy link
Member

EgorBo commented Jan 4, 2023

cc @EgorBo

I would not bother since there is no clear suspect, on the global scale it looks like a bimodal behavior so I'd close this

image

@EgorBo EgorBo closed this as completed Jan 4, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Feb 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI tenet-performance Performance related issue tenet-performance-benchmarks Issue from performance benchmark
Projects
None yet
Development

No branches or pull requests

3 participants