Skip to content

Commit c696fe8

Browse files
committed
Avoid Allocating RowDimension Unneccesarily
This PR builds on PR PHPOffice#3527, introduced but subsequently closed by @goetas. The observation was that AutoFilter did not need to allocate a new RowDimension when the row was not to be filtered. While vetting the PR, it became apparent that Xlsx Writer also allocates RowDimension unnecessarily, with some minor adjustments possible for Ods and Mpdf as well. My tests confirm the initial observation that there can be a considerable memory savings when RowDimension is allocated only when needed. So, even though the original PR is withdrawn, there seems to be value in proceeding with it anyhow.
1 parent d6e2e24 commit c696fe8

File tree

7 files changed

+28
-5
lines changed

7 files changed

+28
-5
lines changed

src/PhpSpreadsheet/Worksheet/AutoFilter.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1049,7 +1049,11 @@ public function showHideRows()
10491049
}
10501050
}
10511051
// Set show/hide for the row based on the result of the autoFilter result
1052-
$this->workSheet->getRowDimension((int) $row)->setVisible($result);
1052+
// If the RowDimension object has not been allocated yet and the row should be visible,
1053+
// then we can avoid any operation since the rows are visible by default (saves a lot of memory)
1054+
if ($result === false || $this->workSheet->rowDimensionExists((int) $row)) {
1055+
$this->workSheet->getRowDimension((int) $row)->setVisible($result);
1056+
}
10531057
}
10541058
$this->evaluated = true;
10551059

src/PhpSpreadsheet/Worksheet/Worksheet.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3874,4 +3874,9 @@ public static function nameRequiresQuotes(string $sheetName): bool
38743874
{
38753875
return preg_match(self::SHEET_NAME_REQUIRES_NO_QUOTES, $sheetName) !== 1;
38763876
}
3877+
3878+
public function isRowVisible(int $row): bool
3879+
{
3880+
return !$this->rowDimensionExists($row) || $this->getRowDimension($row)->getVisible();
3881+
}
38773882
}

src/PhpSpreadsheet/Writer/Html.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -504,7 +504,7 @@ public function generateSheetData()
504504
$html .= $startTag;
505505

506506
// Write row if there are HTML table cells in it
507-
$mpdfInvisible = $this->isMPdf && $sheet->getRowDimension($row)->getVisible() === false;
507+
$mpdfInvisible = $this->isMPdf && !$sheet->isRowVisible($row);
508508
if (!$mpdfInvisible && !isset($this->isSpannedRow[$sheet->getParent()->getIndex($sheet)][$row])) {
509509
// Start a new rowData
510510
$rowData = [];

src/PhpSpreadsheet/Writer/Ods/Content.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ private function writeRows(XMLWriter $objWriter, Worksheet $sheet, int $sheetInd
171171
$objWriter->endElement();
172172
$span_row = 0;
173173
} else {
174-
if ($sheet->getRowDimension($row->getRowIndex())->getRowHeight() > 0) {
174+
if ($sheet->rowDimensionExists($row->getRowIndex()) && $sheet->getRowDimension($row->getRowIndex())->getRowHeight() > 0) {
175175
$objWriter->writeAttribute(
176176
'table:style-name',
177177
sprintf('%s_%d_%d', Style::ROW_STYLE_PREFIX, $sheetIndex, $row->getRowIndex())

src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use PhpOffice\PhpSpreadsheet\Style\Conditional;
1414
use PhpOffice\PhpSpreadsheet\Style\ConditionalFormatting\ConditionalDataBar;
1515
use PhpOffice\PhpSpreadsheet\Style\ConditionalFormatting\ConditionalFormattingRuleExtension;
16+
use PhpOffice\PhpSpreadsheet\Worksheet\RowDimension;
1617
use PhpOffice\PhpSpreadsheet\Worksheet\SheetView;
1718
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet as PhpspreadsheetWorksheet;
1819

@@ -1143,11 +1144,12 @@ private function writeSheetData(XMLWriter $objWriter, PhpspreadsheetWorksheet $w
11431144
}
11441145

11451146
$currentRow = 0;
1147+
$emptyDimension = new RowDimension();
11461148
while ($currentRow++ < $highestRow) {
11471149
$isRowSet = isset($cellsByRow[$currentRow]);
11481150
if ($isRowSet || $worksheet->rowDimensionExists($currentRow)) {
11491151
// Get row dimension
1150-
$rowDimension = $worksheet->getRowDimension($currentRow);
1152+
$rowDimension = $worksheet->rowDimensionExists($currentRow) ? $worksheet->getRowDimension($currentRow) : $emptyDimension;
11511153

11521154
// Write current row?
11531155
$writeCurrentRow = $isRowSet || $rowDimension->getRowHeight() >= 0 || $rowDimension->getVisible() === false || $rowDimension->getCollapsed() === true || $rowDimension->getOutlineLevel() > 0 || $rowDimension->getXfIndex() !== null;

tests/PhpSpreadsheetTests/Worksheet/AutoFilter/AutoFilterCustomNumericTest.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ public function testEqualsListSimple(): void
9595
public function testEqualsList(): void
9696
{
9797
$sheet = $this->initSheet();
98+
$sheet->getRowDimension(4)->setRowHeight(25);
9899
$maxRow = $this->maxRow;
99100
$autoFilter = $sheet->getAutoFilter();
100101
$autoFilter->setRange("A1:A$maxRow");
@@ -115,6 +116,17 @@ public function testEqualsList(): void
115116
->setRuleType(Rule::AUTOFILTER_RULETYPE_CUSTOMFILTER);
116117

117118
self::assertEquals([3, 4, 9, 10], $this->getVisible());
119+
self::assertTrue($sheet->rowDimensionExists(2));
120+
self::assertFalse($sheet->rowDimensionExists(3), 'row visible by default');
121+
self::assertTrue($sheet->rowDimensionExists(4), 'row is visible but height has been set');
122+
self::assertTrue($sheet->rowDimensionExists(5));
123+
self::assertTrue($sheet->rowDimensionExists(6));
124+
self::assertTrue($sheet->rowDimensionExists(7));
125+
self::assertTrue($sheet->rowDimensionExists(8));
126+
self::assertFalse($sheet->rowDimensionExists(9), 'row visible by default');
127+
self::assertFalse($sheet->rowDimensionExists(10), 'row visible by default');
128+
self::assertTrue($sheet->rowDimensionExists(11));
129+
self::assertTrue($sheet->rowDimensionExists(12));
118130
}
119131

120132
public function testNotEqualsList(): void

tests/PhpSpreadsheetTests/Worksheet/AutoFilter/SetupTeardown.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public function getVisibleSheet(Worksheet $sheet): array
6262
$sheet->getAutoFilter()->showHideRows();
6363
$actualVisible = [];
6464
for ($row = 2; $row <= $this->maxRow; ++$row) {
65-
if ($sheet->getRowDimension($row)->getVisible()) {
65+
if ($sheet->isRowVisible($row)) {
6666
$actualVisible[] = $row;
6767
}
6868
}

0 commit comments

Comments
 (0)