Skip to content

Reduced some allocations in QRCodeGenerator (NETCORE_APP only) #595

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.

Sign up for GitHub

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Reduced some allocations in QRCodeGenerator (NETCORE_APP only) #595

Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
804d4b9
Avoid allocation of gluedPolynoms when possible
gfoidl Jun 16, 2025
29d9e0e
Avoid using Linq in MultiplyAlphaPolynoms
gfoidl Jun 16, 2025
bde87ee
Avoid Dictionary allocations in GetNotUniqueExponents
gfoidl Jun 16, 2025
59b2690
Added comment to GetNotUniqueExponents on how it works
gfoidl Jun 17, 2025
58480c9
GaloisField.ShrinkAlphaExp use bit-hacks instead of division / modulo
gfoidl Jun 17, 2025
d694875
CodewordBlock uses a pooled byte-array
gfoidl Jun 17, 2025
9baf6ce
Avoid allocating closures in CapacityTables
gfoidl Jun 17, 2025
38277b1
Avoid allocation closure in QRCodeGenerator
gfoidl Jun 17, 2025
8bc3c30
Use a simple cache for the list codeWordWithECC
gfoidl Jun 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 54 additions & 6 deletions QRCoder/QRCodeGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#endif
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Globalization;
using System.Linq;
using System.Runtime.CompilerServices;
Expand Down Expand Up @@ -1017,8 +1018,15 @@ private static Polynom MultiplyAlphaPolynoms(Polynom polynomBase, Polynom polyno
}

// Identify and merge terms with the same exponent.
#if NETCOREAPP
var toGlue = GetNotUniqueExponents(resultPolynom, resultPolynom.Count <= 128 ? stackalloc int[128].Slice(0, resultPolynom.Count) : new int[resultPolynom.Count]);
var gluedPolynoms = toGlue.Length <= 128
? stackalloc PolynomItem[128].Slice(0, toGlue.Length)
: new PolynomItem[toGlue.Length];
#else
var toGlue = GetNotUniqueExponents(resultPolynom);
var gluedPolynoms = new PolynomItem[toGlue.Length];
#endif
var gluedPolynomsIndex = 0;
foreach (var exponent in toGlue)
{
Expand All @@ -1036,7 +1044,11 @@ private static Polynom MultiplyAlphaPolynoms(Polynom polynomBase, Polynom polyno

// Remove duplicated exponents and add the corrected ones back.
for (int i = resultPolynom.Count - 1; i >= 0; i--)
#if NETCOREAPP
if (toGlue.Contains(resultPolynom[i].Exponent))
#else
if (Array.IndexOf(toGlue, resultPolynom[i].Exponent) >= 0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before Linq was used w/ it's generic Contains method.

  • for AOT Linq requires way more code
  • it's an interface dispatch that isn't needed

Note: for .NET (Core) the span-based Contains is used.

#endif
resultPolynom.RemoveAt(i);
foreach (var polynom in gluedPolynoms)
resultPolynom.Add(polynom);
Expand All @@ -1046,20 +1058,55 @@ private static Polynom MultiplyAlphaPolynoms(Polynom polynomBase, Polynom polyno
return resultPolynom;

// Auxiliary function to identify exponents that appear more than once in the polynomial.
int[] GetNotUniqueExponents(Polynom list)
#if NETCOREAPP
static ReadOnlySpan<int> GetNotUniqueExponents(Polynom list, Span<int> buffer)
{
var dic = new Dictionary<int, bool>(list.Count);
Debug.Assert(list.Count == buffer.Length);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you plan to leave the Debug.Assert code in here? Just wondering; it will get removed from release builds anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like to have some Debug.Asserts

  • they check some invariants in debug builds and while running tests
  • are a bit of self-documenting the intention (so why write a comment, when the assert can also be done?)

Especially here the buffer is passed in as argument and must have the correct size.
If the size doesn't match, then the tests (under debug) will fail and one knows why. Otherwise it may be hard to track down the bug.

So I'd leave them in the code.

As you said: for !DEBUG these asserts won't have any effect.


int idx = 0;
foreach (var row in list)
{
#if NETCOREAPP
if (dic.TryAdd(row.Exponent, false))
dic[row.Exponent] = true;
buffer[idx++] = row.Exponent;
}

buffer.Sort();

idx = 0;
int expCount = 0;
int last = buffer[0];

for (int i = 1; i < buffer.Length; ++i)
{
if (buffer[i] == last)
{
expCount++;
}
else
{
if (expCount > 0)
{
Debug.Assert(idx <= i - 1);

buffer[idx++] = last;
expCount = 0;
}
}

last = buffer[i];
}

return buffer.Slice(0, idx);
}
#else
static int[] GetNotUniqueExponents(Polynom list)
{
var dic = new Dictionary<int, bool>(list.Count);
foreach (var row in list)
{
if (!dic.ContainsKey(row.Exponent))
dic.Add(row.Exponent, false);
else
dic[row.Exponent] = true;
#endif
}

// Collect all exponents that appeared more than once.
Expand All @@ -1080,6 +1127,7 @@ int[] GetNotUniqueExponents(Polynom list)

return result;
}
#endif
}

/// <inheritdoc cref="IDisposable.Dispose"/>
Expand Down
Loading