-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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.
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
Conversation
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. |
The reduction in allocations is very impressive! |
I fixed the CI scripts in #592 btw |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! (subject to tests passing, of course)
Honestly yes… |
Found some more allocation that can be quite easily avoided.
(note: time column isn't really comparable to results in top post (different machine), so only allocations count here) |
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? Thanks again for your effort and contribution! |
As written in #605 (comment) 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 if you merge in master, tests should run properly now |
Profiling showed that there are some allocations in
QRCodeGenerator
that can be quite easily avoided.simple console app for profiling
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
After
Benchmarks
Before
After