Reduce memory consumption of Worksheet::rangeToArray() when many empty rows are read #3834
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@sj-i This is very impressive analysis and an ingenious solution. I need to study it a little more to understand it better, but my intention is to merge it within the next day or two. |
|
Thank you for your contribution, and for the lesson about CoW. |
oleibman
added a commit
to oleibman/PhpSpreadsheet
that referenced
this pull request
Dec 21, 2023
There are severe inefficiencies in `toArray` (or, to be more precise, `rangeToArray`, which toArray calls). For small ranges, the inefficiencies hardly matter, and, for large ranges, the code was likely to exhaust memory before the execution speed mattered. But, with PR PHPOffice#3834 now merged into the codebase, the memory problem is much lessened, and there is now merit in looking into performance. The code currently accesses each cell in the range. This PR changes it to access only cells which have been initialized - fewer cells processed, and many fewer function calls for those that are. For ranges which are densely filled, there will be little difference. However, for ranges which are lightly filled (a very common occurrence in issues which are raised for this project), the effect will be staggering. The sample file which accompanied 3834 saw an enormous reduction in memory consumption from 20GB to 32MB when toArray was used on it. The same file still took well over an hour to process even after the merge; after this PR, it is processed in approximately a second. While testing this change, it became apparent that Collection/Cells does not handle column XFD (highest allowed) correctly. That is corrected and new tests added.
12 tasks
oleibman
added a commit
to oleibman/PhpSpreadsheet
that referenced
this pull request
Feb 17, 2024
Fix PHPOffice#3904. PR PHPOffice#3839 provided a huge performance boost for sparsely populated spreadsheets. Unfortunately, it degraded performance for more densely populated spreadsheets when writing Csvs. The reason is that Csv Writer calls toArray for each row, meaning that a lot of the intermediate data used to speed things up needs to be recalculated for every row. It would be better off calling toArray just once for the entire spreadsheet; however this gives back some of the memory improvements of PR PHPOffice#3834. However, the memory effects can be substantially alleviated by supplying a Generator function to do the work. This PR does that; the result is that Csv Writer is now quite a bit faster, and with only a small memory uptick, vs. its performance in PhpSpreadsheet 1.29.
11 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is:
Checklist:
Why this change is needed?
There may be cases where some data is located "far away" in a sheet, like
AMJ1048576, and the rest of the rows are mostly empty. This change dramatically reduces the memory usage in such cases. For example, currently usingWorksheet::toArray()withexample.xlsxin #3814 eats up over 20GB of memory in PHP 8.2. After this change, the memory consumption becomes less than 32MB.So, this fixes #3814