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

Update to net8 #2583

Merged
merged 20 commits into from
Dec 4, 2023
Merged

Update to net8 #2583

merged 20 commits into from
Dec 4, 2023

Conversation

stefannikolei
Copy link
Contributor

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

@stefannikolei stefannikolei marked this pull request as ready for review November 19, 2023 10:11
src/ImageSharp.ruleset Outdated Show resolved Hide resolved
@@ -1,7 +1,11 @@
<?xml version="1.0" encoding="utf-8"?>
<RuleSet Name="ImageSharp" ToolsVersion="17.0">
<Include Path="..\shared-infrastructure\sixlabors.ruleset" Action="Default" />
<Rules AnalyzerId="Microsoft.CodeAnalysis.CSharp.NetAnalyzers" RuleNamespace="Microsoft.CodeAnalysis.CSharp.NetAnalyzers">
<Rule Id="CA1857" Action="None" />
<Rule Id="CA1859" Action="None" />
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://learn.microsoft.com/de-de/dotnet/fundamentals/code-analysis/quality-rules/ca1859

I disabled this to get a build without errors. I can remove it and fix all places

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should open up a discussion or issue to track it so it's not forgotten.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just had a hack at this. Only 3 changes required in private methods.

src/ImageSharp/Formats/ImageFormatManager.cs Show resolved Hide resolved
@JimBobSquarePants
Copy link
Member

Thanks for this. You might want to put a pause on it though until we ship v3.1 We'll need to update SharedInfrastructure submodule following that.

@stefannikolei
Copy link
Contributor Author

@JimBobSquarePants should we run the arm tests before we merge this?

@JimBobSquarePants JimBobSquarePants added enhancement breaking Signifies a binary breaking change. arch:arm64 labels Nov 29, 2023
@JimBobSquarePants
Copy link
Member

@stefannikolei triggered.

Copy link
Member

@JimBobSquarePants JimBobSquarePants left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks great to me. Let's pull the trigger!

@JimBobSquarePants JimBobSquarePants merged commit 3d69f21 into SixLabors:main Dec 4, 2023
6 checks passed
@stefannikolei
Copy link
Contributor Author

I just ran the JpegEncoder tests against .NET6 and .NET8. Those look promising.





.NET8

BenchmarkDotNet v0.13.10, macOS Sonoma 14.1.2 (23B92) [Darwin 23.1.0]
Intel Core i9-9880H CPU 2.30GHz, 1 CPU, 16 logical and 8 physical cores
.NET SDK 8.0.100
  [Host]     : .NET 8.0.0 (8.0.23.53103), X64 RyuJIT AVX2
  DefaultJob : .NET 8.0.0 (8.0.23.53103), X64 RyuJIT AVX2


| Method                              | Mean      | Error     | StdDev    |
|------------------------------------ |----------:|----------:|----------:|
| 'Baseline 4:4:4 Interleaved'        | 11.185 ms | 0.3432 ms | 0.9957 ms |
| 'Baseline 4:2:0 Interleaved'        |  8.775 ms | 0.1736 ms | 0.4355 ms |
| 'Baseline 4:0:0 (grayscale)'        |  1.254 ms | 0.0173 ms | 0.0135 ms |
| 'Progressive 4:2:0 Non-Interleaved' | 11.796 ms | 0.2352 ms | 0.3297 ms |


.NET6

BenchmarkDotNet=v0.13.0, OS=macOS 14.1.2 (23B92) [Darwin 23.1.0]
Intel Core i9-9880H CPU 2.30GHz, 1 CPU, 16 logical and 8 physical cores
.NET SDK=8.0.100
  [Host]     : .NET 6.0.21 (6.0.2123.36311), X64 RyuJIT
  DefaultJob : .NET 6.0.21 (6.0.2123.36311), X64 RyuJIT


|                              Method |      Mean |     Error |    StdDev |
|------------------------------------ |----------:|----------:|----------:|
|        'Baseline 4:4:4 Interleaved' | 18.003 ms | 0.3533 ms | 0.6636 ms |
|        'Baseline 4:2:0 Interleaved' | 11.986 ms | 0.2339 ms | 0.3279 ms |
|        'Baseline 4:0:0 (grayscale)' |  2.181 ms | 0.0398 ms | 0.0426 ms |
| 'Progressive 4:2:0 Non-Interleaved' | 16.453 ms | 0.3239 ms | 0.4211 ms |

@JimBobSquarePants
Copy link
Member

Just did a quick test using the new V4 alpha comparing against other frameworks. Lots of good, free performance there.

I can't wait to get stuck in and having a go implementing as much of the new SIMD APIs as possible.

image

@stefannikolei stefannikolei deleted the sn/net8 branch January 23, 2024 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch:arm64 breaking Signifies a binary breaking change. enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants