-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Optimize ColumnNumberToName function performance, reduce about 50% memory usage and 50% time cost #1935
Conversation
Optimize memory allocation by pre-allocating enough space for the resulting string.
… creation sheet time Optimize memory by using when SetCell is called many times, the slice grows and allocates a lot of memory each time it reaches its cap limit.
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.
Thanks for your PR. I've left some comments.
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.
I suggest that we separate these two improvements into 2 PRs, and I will take a reconsider about the design of reallocate rows for worksheets.
Sure, I will separate these two changes into different PRs. Do I need to create two new PRs, or can I keep the changes related to the ColumnNumberToName function in this PR? Thanks for reviewing! |
You can keep the changes related to the |
Done, I left in this PR only the changes related to the ColumnNumberToName function. Thanks. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1935 +/- ##
=======================================
Coverage 99.30% 99.30%
=======================================
Files 32 32
Lines 24961 24965 +4
=======================================
+ Hits 24787 24791 +4
Misses 93 93
Partials 81 81
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
LGTM, thanks for your contribution.
…mory usage and 50% time cost (qax-os#1935) Co-authored-by: zhayt <[email protected]>
PR Details
Optimizing memory usage by pre-allocation.
Description
ColumnNumberToName Function:
ColumnNumberToName
function previously concatenated rows using the+
operator, leading to significant memory allocation when called frequently.Sheet Pre-allocation:
xlsxWorksheet
structure now contains a field with a slice type. When calling theSetCell...
function to add a row to the sheet, it in turn calls theprepareSheetXML
function. This function adds anxlsxRow
structure to the slice.prepareSheetXML
caused the slice to grow, allocating a lot of memory and performing unnecessary copying. The new approach pre-allocates the maximum number of rows, reducing memory allocations and copying overhead.Related Issue
Motivation and Context
How Has This Been Tested
Types of changes
Checklist
Feel free to modify any part of this to better fit your needs!