From 827662c49ee61aed68c8d90e1944ca54efff510f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Einar=20Gangs=C3=B8?= Date: Sun, 10 Jan 2021 13:31:02 +0100 Subject: [PATCH] Optimize applyFromArray by caching existing styles Prevent calling clone and getHashCode when not needed because these calls are very expensive. When applying styles to a range of cells can we cache the styles we encounter along the way so we don't need to look them up with getHashCode later. --- CHANGELOG.md | 2 +- src/PhpSpreadsheet/Style/Style.php | 61 ++++++++++++++++++- tests/PhpSpreadsheetTests/Style/StyleTest.php | 23 +++++++ 3 files changed, 82 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4741afe0a..811f7d8052 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Changed -- Nothing. +- Improved performance of Style::applyFromArray() when applied to several cells. [#1785](https://github.com/PHPOffice/PhpSpreadsheet/issues/1785). ### Deprecated diff --git a/src/PhpSpreadsheet/Style/Style.php b/src/PhpSpreadsheet/Style/Style.php index f7c1be23fc..a31c72b16f 100644 --- a/src/PhpSpreadsheet/Style/Style.php +++ b/src/PhpSpreadsheet/Style/Style.php @@ -63,6 +63,21 @@ class Style extends Supervisor */ protected $quotePrefix = false; + /** + * Cloning a Style and calling getHashCode() is expensive. + * When applying a single style to several cells can we cache the styles + * we encounter so to prevent calling these methods in order to determine + * if they will end up being the same as an existing style. + * + * The key is the hashcode of an old style + * The value is the old style with the new styles applied + * + * @see Style::applyFromArray() + * + * @var array + */ + private static $cachedStyles; + /** * Create a new Style. * @@ -351,6 +366,8 @@ public function applyFromArray(array $pStyles, $pAdvanced = true) // First loop through columns, rows, or cells to find out which styles are affected by this operation switch ($selectionType) { case 'COLUMN': + self::$cachedStyles = []; + $oldXfIndexes = []; for ($col = $rangeStart[0]; $col <= $rangeEnd[0]; ++$col) { $oldXfIndexes[$this->getActiveSheet()->getColumnDimensionByColumn($col)->getXfIndex()] = true; @@ -363,8 +380,12 @@ public function applyFromArray(array $pStyles, $pAdvanced = true) } } + self::$cachedStyles = null; + break; case 'ROW': + self::$cachedStyles = []; + $oldXfIndexes = []; for ($row = $rangeStart[1]; $row <= $rangeEnd[1]; ++$row) { if ($this->getActiveSheet()->getRowDimension($row)->getXfIndex() == null) { @@ -381,6 +402,8 @@ public function applyFromArray(array $pStyles, $pAdvanced = true) } } + self::$cachedStyles = null; + break; case 'CELL': $oldXfIndexes = []; @@ -393,14 +416,46 @@ public function applyFromArray(array $pStyles, $pAdvanced = true) break; } + // $cachedStyles is set when applying style for a range of cells, either column or row + $isUsingCache = self::$cachedStyles !== null; + // clone each of the affected styles, apply the style array, and add the new styles to the workbook $workbook = $this->getActiveSheet()->getParent(); foreach ($oldXfIndexes as $oldXfIndex => $dummy) { $style = $workbook->getCellXfByIndex($oldXfIndex); - $newStyle = clone $style; - $newStyle->applyFromArray($pStyles); - if ($existingStyle = $workbook->getCellXfByHashCode($newStyle->getHashCode())) { + if (!$isUsingCache) { + // Clone the old style and apply array, then see if this style already exist + $newStyle = clone $style; + $newStyle->applyFromArray($pStyles); + + $existingStyle = $workbook->getCellXfByHashCode($newStyle->getHashCode()); + } else { + $objId = spl_object_id($style); + + // Check if we have already called getHashCode on the $oldXfIndex + $styleHash = self::$cachedStyles['hashes'][$objId] ?? null; + if ($styleHash === null) { + // Cache hashCode of $oldXfIndex so we do not need to call this again + $styleHash = self::$cachedStyles['hashes'][$objId] = $style->getHashCode(); + } + + $existingStyle = self::$cachedStyles[$styleHash] ?? null; + + if ($existingStyle === null) { + // The $oldXfIndex combined with the style array does not exist, so we create it now + $newStyle = clone $style; + $newStyle->applyFromArray($pStyles); + + $existingStyle = $workbook->getCellXfByHashCode($newStyle->getHashCode()); + + // Cache the Style which is a result of $oldXfIndex and the style array + self::$cachedStyles[$styleHash] = $existingStyle ?: $newStyle; + self::$cachedStyles['hashes'][$objId] = $styleHash; + } + } + + if ($existingStyle) { // there is already such cell Xf in our collection $newXfIndexes[$oldXfIndex] = $existingStyle->getIndex(); } else { diff --git a/tests/PhpSpreadsheetTests/Style/StyleTest.php b/tests/PhpSpreadsheetTests/Style/StyleTest.php index 6f1577099a..3d20b044c5 100644 --- a/tests/PhpSpreadsheetTests/Style/StyleTest.php +++ b/tests/PhpSpreadsheetTests/Style/StyleTest.php @@ -55,6 +55,29 @@ public function testStyleColumn(): void self::assertFalse($sheet->getStyle('C3')->getFont()->getItalic()); } + public function testStyleIsReused(): void + { + $spreadsheet = new Spreadsheet(); + $sheet = $spreadsheet->getActiveSheet(); + $styleArray = [ + 'font' => [ + 'italic' => true, + ], + ]; + + $sheet->getStyle('A1')->getFont()->setBold(true); + $sheet->getStyle('A2')->getFont()->setBold(true); + $sheet->getStyle('A3')->getFont()->setBold(true); + $sheet->getStyle('A3')->getFont()->setItalic(true); + + $sheet->getStyle('A')->applyFromArray($styleArray); + + self::assertCount(4, $spreadsheet->getCellXfCollection()); + $spreadsheet->garbageCollect(); + + self::assertCount(3, $spreadsheet->getCellXfCollection()); + } + public function testStyleRow(): void { $spreadsheet = new Spreadsheet();