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.
PHPOffice · Jul 14, 2021 · d3c7ef4 · d3c7ef4
1 parent 418cd30
commit d3c7ef4
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com)
 and this project adheres to [Semantic Versioning](https://semver.org).
 
+## Unreleased - TBD
+
+### Changed
+
+- Improved performance of Style::applyFromArray() when applied to several cells. [#1785](https://github.com/PHPOffice/PhpSpreadsheet/issues/1785).
+
 ## 1.18.0 - 2021-05-31
 
 ### Added

diff --git 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<string, Style|Style[]>
+     */
+    private static $cachedStyles;
+
     /**
      * Create a new Style.
      *
@@ -350,15 +365,47 @@ public function applyFromArray(array $pStyles, $pAdvanced = true)
             // First loop through columns, rows, or cells to find out which styles are affected by this operation
             $oldXfIndexes = $this->getOldXfIndexes($selectionType, $rangeStartIndexes, $rangeEndIndexes, $columnStart, $columnEnd, $pStyles);
 
+            // $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();
             $newXfIndexes = [];
             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
@@ -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();