diff --git a/CHANGELOG.md b/CHANGELOG.md index 163e79a5d..97ef5be4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,7 @@ Some earlier branches remain supported and security fixes are applied to them; i ### Changed -- Nothing yet. +- Performance: avoid `Worksheet::getStyle()` on every `Cell::setValueExplicit()` unless quote-prefix must change. On a dense 40k-cell populate+save microbenchmark this cut wall time by ~5%. ### Moved diff --git a/src/PhpSpreadsheet/Cell/Cell.php b/src/PhpSpreadsheet/Cell/Cell.php index 2e5d87bf0..dd9b53310 100644 --- a/src/PhpSpreadsheet/Cell/Cell.php +++ b/src/PhpSpreadsheet/Cell/Cell.php @@ -370,16 +370,16 @@ class Cell implements Stringable $worksheet = $this->getWorksheet(); $spreadsheet = $worksheet->getParent(); if (isset($spreadsheet) && $spreadsheet->getIndex($worksheet, true) >= 0) { - $originalSelected = $worksheet->getSelectedCells(); - $activeSheetIndex = $spreadsheet->getActiveSheetIndex(); - $style = $this->getStyle(); - $oldQuotePrefix = $style->getQuotePrefix(); + // Avoid Worksheet::getStyle() (selection + validation) unless quotePrefix must change. + $oldQuotePrefix = $spreadsheet->getCellXfByIndex($this->getXfIndex())->getQuotePrefix(); if ($oldQuotePrefix !== $quotePrefix) { - $style->setQuotePrefix($quotePrefix); - } - $worksheet->setSelectedCells($originalSelected); - if ($activeSheetIndex >= 0) { - $spreadsheet->setActiveSheetIndex($activeSheetIndex); + $originalSelected = $worksheet->getSelectedCells(); + $activeSheetIndex = $spreadsheet->getActiveSheetIndex(); + $this->getStyle()->setQuotePrefix($quotePrefix); + $worksheet->setSelectedCells($originalSelected); + if ($activeSheetIndex >= 0) { + $spreadsheet->setActiveSheetIndex($activeSheetIndex); + } } } diff --git a/tests/Benchmark/LargeXlsxWriteBenchmarkTest.php b/tests/Benchmark/LargeXlsxWriteBenchmarkTest.php new file mode 100644 index 000000000..df6002d99 --- /dev/null +++ b/tests/Benchmark/LargeXlsxWriteBenchmarkTest.php @@ -0,0 +1,69 @@ +getActiveSheet(); + for ($row = 1; $row <= self::ROWS; ++$row) { + for ($col = 1; $col <= self::COLS; ++$col) { + $sheet->getCell([$col, $row])->setValue("R{$row}C{$col}"); + } + } + + $writer = new Xlsx($spreadsheet); + $writer->setPreCalculateFormulas(false); + $writer->save($filename); + + $elapsedMs = (hrtime(true) - $start) / 1e6; + $peakMiB = memory_get_peak_usage(true) / (1024 * 1024); + $deltaMiB = (memory_get_usage(true) - $memBefore) / (1024 * 1024); + $cellCount = self::ROWS * self::COLS; + + fwrite( + STDERR, + sprintf( + "LargeXlsxWriteBenchmark: %d cells, %.1f ms, peak=%.1f MiB, delta=%.1f MiB, size=%.1f KiB\n", + $cellCount, + $elapsedMs, + $peakMiB, + $deltaMiB, + filesize($filename) / 1024 + ) + ); + + self::assertFileExists($filename); + self::assertGreaterThan(1000, filesize($filename)); + + $spreadsheet->disconnectWorksheets(); + @unlink($filename); + } +} diff --git a/tests/PhpSpreadsheetTests/Cell/SetValueExplicitQuotePrefixPerfTest.php b/tests/PhpSpreadsheetTests/Cell/SetValueExplicitQuotePrefixPerfTest.php new file mode 100644 index 000000000..525bb54d8 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Cell/SetValueExplicitQuotePrefixPerfTest.php @@ -0,0 +1,70 @@ +getActiveSheet(); + $sheet->setSelectedCells('Z99'); + $spreadsheet->createSheet()->setTitle('Other'); + $spreadsheet->setActiveSheetIndex(1); + + $cell = $sheet->getCell('A1'); + $cell->setValueExplicit('=not-a-formula', DataType::TYPE_STRING); + + // Read quotePrefix from the cell xf without going through Worksheet::getStyle() + // (which would change the selection). + $xf = $spreadsheet->getCellXfByIndex($sheet->getCell('A1')->getXfIndex()); + self::assertTrue($xf->getQuotePrefix()); + self::assertSame('Z99', $sheet->getSelectedCells()); + self::assertSame(1, $spreadsheet->getActiveSheetIndex()); + + $spreadsheet->disconnectWorksheets(); + } + + public function testQuotePrefixNotAppliedForNormalString(): void + { + $spreadsheet = new Spreadsheet(); + $sheet = $spreadsheet->getActiveSheet(); + $sheet->getCell('A1')->setValueExplicit('hello', DataType::TYPE_STRING); + + $xf = $spreadsheet->getCellXfByIndex($sheet->getCell('A1')->getXfIndex()); + self::assertFalse($xf->getQuotePrefix()); + + $spreadsheet->disconnectWorksheets(); + } + + public function testQuotePrefixClearedWhenReplacingPrefixedString(): void + { + $spreadsheet = new Spreadsheet(); + $sheet = $spreadsheet->getActiveSheet(); + $sheet->getCell('A1')->setValueExplicit('=prefixed', DataType::TYPE_STRING); + self::assertTrue($spreadsheet->getCellXfByIndex($sheet->getCell('A1')->getXfIndex())->getQuotePrefix()); + + $sheet->getCell('A1')->setValueExplicit('plain', DataType::TYPE_STRING); + self::assertFalse($spreadsheet->getCellXfByIndex($sheet->getCell('A1')->getXfIndex())->getQuotePrefix()); + + $spreadsheet->disconnectWorksheets(); + } + + public function testNumericValueDoesNotTouchQuotePrefix(): void + { + $spreadsheet = new Spreadsheet(); + $sheet = $spreadsheet->getActiveSheet(); + $sheet->getCell('A1')->setValueExplicit(42, DataType::TYPE_NUMERIC); + + self::assertFalse($spreadsheet->getCellXfByIndex($sheet->getCell('A1')->getXfIndex())->getQuotePrefix()); + self::assertSame(42, $sheet->getCell('A1')->getValue()); + + $spreadsheet->disconnectWorksheets(); + } +}