QRCoder icon indicating copy to clipboard operation
QRCoder copied to clipboard

Reduced some allocations in `QRCodeGenerator` (NETCORE_APP only)

Open gfoidl opened this issue 5 months ago • 6 comments

Profiling showed that there are some allocations in QRCodeGenerator that can be quite easily avoided.

simple console app for profiling
using System.Diagnostics;
using QRCoder;

string payload = $"""
    Time    : {DateTimeOffset.Now:O}
    Machine : {Environment.MachineName}
    Activity: {Activity.Current?.RootId}
    """;

int len = 0;

for (int i = 0; i < 1_000; ++i)
{
    string imgSrc = GetQRCode("sdfsdf");
    len += imgSrc.Length;
}

Console.WriteLine(len);

static string GetQRCode(string payload)
{
    using QRCodeGenerator qrCodeGenerator = new();
    using QRCodeData qrCodeData = qrCodeGenerator.CreateQrCode(payload, QRCodeGenerator.ECCLevel.Q);
    using Base64QRCode qrCodeBase64 = new(qrCodeData);
    string base64QRCode = qrCodeBase64.GetGraphic(20);
    return $"data:image/png;base64,{base64QRCode}";
}

Allocations are removed / avoided for:

  • Dictionary<,>.Entry[]
  • QRCodeGenerator.PolynomItem[]
  • Dictionary<,>

The change is done only for .NET (Core) targets, as Span<T> is used. By adding a reference to System.Memory package this change could also be done for .NET Desktop.

Profile

Before

before

After

after

Benchmarks

Before

| Method              | Mean        | Error     | StdDev    | Gen0   | Allocated |
|-------------------- |------------:|----------:|----------:|-------:|----------:|
| CreateQRCode        |    235.2 μs |   4.50 μs |   5.00 μs | 2.1973 |    7.2 KB |
| CreateQRCodeLong    |  2,684.9 μs |  48.00 μs |  44.89 μs | 7.8125 |  33.25 KB |
| CreateQRCodeLongest | 16,264.4 μs | 317.90 μs | 485.47 μs |      - |  79.69 KB |

After

| Method              | Mean        | Error     | StdDev    | Gen0   | Allocated |
|-------------------- |------------:|----------:|----------:|-------:|----------:|
| CreateQRCode        |    232.0 μs |   4.63 μs |   6.64 μs | 0.9766 |   4.38 KB |
| CreateQRCodeLong    |  2,574.1 μs |  41.97 μs |  39.26 μs | 3.9063 |     12 KB |
| CreateQRCodeLongest | 15,573.7 μs | 292.90 μs | 259.65 μs |      - |  48.24 KB |

gfoidl avatar Jun 16 '25 21:06 gfoidl

By adding a reference to System.Memory package this change could also be done for .NET Desktop.

I wouldn't. There has been so many performance enhancements in .NET Core since .NET Framework, that if anyone wants better performance, they should use .NET Core. And I'm sure the fallback performance is plenty good enough anyway.

Shane32 avatar Jun 16 '25 22:06 Shane32

The reduction in allocations is very impressive!

Shane32 avatar Jun 16 '25 22:06 Shane32

I fixed the CI scripts in #592 btw

Shane32 avatar Jun 16 '25 22:06 Shane32

I fixed the CI scripts in #592 btw

Should that change be separated into a own PR to fix CI?

Otherwise every PR has to re-do the same until your PR gets merged.


@Shane32 thanks for your review and comments 👍🏻

gfoidl avatar Jun 17 '25 14:06 gfoidl

Should that change be separated into a own PR to fix CI?

Honestly yes…

Shane32 avatar Jun 17 '25 14:06 Shane32

Found some more allocation that can be quite easily avoided.

| Method              | Mean        | Error     | StdDev    | Gen0   | Allocated |
|-------------------- |------------:|----------:|----------:|-------:|----------:|
| CreateQRCode        |    240.0 μs |   4.57 μs |   4.89 μs | 1.2207 |   4.07 KB |
| CreateQRCodeLong    |  2,730.3 μs |  41.85 μs |  37.10 μs |      - |  11.05 KB |
| CreateQRCodeLongest | 17,367.9 μs | 345.33 μs | 657.03 μs |      - |  46.42 KB |

(note: time column isn't really comparable to results in top post (different machine), so only allocations count here)

gfoidl avatar Jun 17 '25 20:06 gfoidl

Hi @gfoidl ,

since QRCoder will reach its end of life and the repository will be archived on November 1st, 2025, I’d like to clarify how to proceed with this PR.

Would you prefer me to merge it before the archival, or should I close it instead?
Please let me know what you think – I want to handle your contribution in the way that feels most appropriate to you.

Thanks again for your effort and contribution!

codebude avatar Sep 27 '25 21:09 codebude

As written in https://github.com/codebude/QRCoder/issues/605#issuecomment-3342962313 I'd like that the repo here continues to live, and don't be archived, so please merge the PR as it brings goodness to your baby QRCoder.

gfoidl avatar Sep 28 '25 11:09 gfoidl

@gfoidl if you merge in master, tests should run properly now

Shane32 avatar Sep 30 '25 12:09 Shane32