search

FileOnQ / Imaging.Heif

A C#/.NET wrapper around libheif for decoding and processing high efficiency image formats (heif, heic).
GNU Lesser General Public License v3.0
15 stars 4 forks source link

Feature/net 6 support #62

Closed kenny-sellers closed 2 years ago

kenny-sellers commented 2 years ago

Fixes: #46

Description

Adding .NET 6 Support

Merge Checklist

SkyeHoefling commented 2 years ago

/benchmark

SkyeHoefling commented 2 years ago

I updated the benchmark project to support running under net48, net5, and net6. After a successful run we will need to update the saved results as the table will be slightly different from what is stored in the repository. The benefit here is we will be able to look at differences between all our supported platforms.

Let's wait and see with the CI says, I noticed locally there is a very small memory leak in net6 and net48 - under 200 bytes

SkyeHoefling commented 2 years ago

benchmarks failed because .NET 6 wasn't installed on job benchmark-run. I fixed this in main, let's try running them again.

SkyeHoefling commented 2 years ago

/benchmark

SkyeHoefling commented 2 years ago

I just peaked at the result of the benchmark build for Thumbnails and it includes net48 and net6 but not net 6. We need to add another ensure .NET SDK command to ensure .NET 5 is installed. See output https://github.com/FileOnQ/Imaging.Heif/actions/runs/1678150679

Method Job Runtime Mean Error StdDev Median Gen 0 Gen 1 Gen 2 Allocated Allocated native memory Native memory leak
Thumbnail_Write Job-RFREVW .NET 5.0 NA NA NA NA - - - - - -
Thumbnail_ToArray Job-RFREVW .NET 5.0 NA NA NA NA - - - - - -
Thumbnail_ToSpan Job-RFREVW .NET 5.0 NA NA NA NA - - - - - -
Thumbnail_ToStream Job-RFREVW .NET 5.0 NA NA NA NA - - - - - -
Thumbnail_Write Job-LTLQYG .NET 6.0 55.58 ms 1.052 ms 1.475 ms 55.03 ms - - - 832 B 5,125,137 B -
Thumbnail_ToArray Job-LTLQYG .NET 6.0 55.67 ms 1.075 ms 1.103 ms 55.67 ms - - - 66,888 B 5,124,325 B -
Thumbnail_ToSpan Job-LTLQYG .NET 6.0 53.05 ms 1.054 ms 1.732 ms 52.23 ms - - - 600 B 5,124,581 B -
Thumbnail_ToStream Job-LTLQYG .NET 6.0 52.51 ms 0.986 ms 0.922 ms 52.56 ms - - - 66,952 B 5,124,581 B -
Thumbnail_Write Job-UALGFG .NET Framework 4.8 53.53 ms 1.062 ms 2.376 ms 52.91 ms - - - 74,504 B 5,125,459 B 292 B
Thumbnail_ToArray Job-UALGFG .NET Framework 4.8 54.69 ms 1.079 ms 2.627 ms 53.92 ms - - - 74,504 B 5,125,163 B 292 B
Thumbnail_ToSpan Job-UALGFG .NET Framework 4.8 53.01 ms 1.035 ms 1.382 ms 52.94 ms - - - 8,192 B 5,125,163 B 292 B
Thumbnail_ToStream Job-UALGFG .NET Framework 4.8 55.46 ms 1.107 ms 2.133 ms 55.69 ms - - - 140,816 B 5,125,163 B 292 B
SkyeHoefling commented 2 years ago

/benchmark

SkyeHoefling commented 2 years ago

@kenny-sellers can you take a look at the memory leak noted for .NET 4.8

kenny-sellers commented 2 years ago

@ahoefling Just for documentation purposes. We are also seeing a memory leak with Primary images. That one is 234B.

SkyeHoefling commented 2 years ago

Thanks for confirming the issue. I don't think we should have the memory leak or benchmarking hold up this PR. @kenny-sellers can you create a new issue to track the memory leak and include the .NET Framework benchmarks to document the problem.

I will make another pass at the code here and see what we need to do to close this out

SkyeHoefling commented 2 years ago

/benchmark

SkyeHoefling commented 2 years ago

As long as the builds and benchmarks succeed, we can merge this PR

SkyeHoefling commented 2 years ago

Benchmark is failing, because we need the script updated in main. We will clean this up in #66. If the current build passes we will merge this PR