-
Couldn't load subscription status.
- Fork 5.2k
Improve performance of reading zip comments #119457
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
Improve performance of reading zip comments #119457
Conversation
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.
Pull Request Overview
This PR improves memory allocation performance in the ZipHelper.GetEncodedTruncatedBytesFromString method by replacing direct byte array allocation with ArrayPool<byte> for temporary storage, reducing garbage collection pressure.
Key Changes
- Replaces
encoding.GetBytes(text)allocation with pooled array rental - Adds proper array pooling with try/finally pattern for resource cleanup
- Maintains existing functionality while reducing memory allocations
src/libraries/System.IO.Compression/src/System/IO/Compression/ZipHelper.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.IO.Compression/src/System/IO/Compression/ZipHelper.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.IO.Compression/src/System/IO/Compression/ZipHelper.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.IO.Compression/src/System/IO/Compression/ZipHelper.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.IO.Compression/src/System/IO/Compression/ZipHelper.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.IO.Compression/src/System/IO/Compression/ZipHelper.cs
Show resolved
Hide resolved
|
Hello @jkotas @stephentoub, I ran some benchmarks to compare the initial method, then the arraypool approach and then using jan's suggestion and I think based on the results, Jan's version offers the best improvements. benchmarks.ods |
Could you post it as text or as a screenshot? Not everyone have installed Libre Office. |
|
Content of benchmarks.ods: Input: string('A', 5000);
Input: string.Concat(Enumerable.Repeat("😀😎🚀🔥", 1000));
Input:string('X', 50_000);
Input: string.Concat(Enumerable.Repeat("X😀XX", 10_000));
|
|
Hey @jkotas @stephentoub, what do you think about these changes? |
Improve byte array allocations for comments reading
Fixes #64767