Improved Performance for toArray

oleibman · oleibman · commit 096e19349575 · 2023-12-20T22:47:10.000-08:00
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.
diff --git a/src/PhpSpreadsheet/Collection/Cells.php b/src/PhpSpreadsheet/Collection/Cells.php
@@ -144,6 +144,18 @@ public function getSortedCoordinates(): array
         return array_keys($this->index);
     }
 
+    /**
+     * Get a sorted list of all cell coordinates currently held in the collection by index (16384*row+column).
+     *
+     * @return int[]
+     */
+    public function getSortedCoordinatesInt(): array
+    {
+        asort($this->index);
+
+        return array_values($this->index);
+    }
+
     /**
      * Return the cell coordinate of the currently active cell object.
      *
@@ -188,9 +200,9 @@ public function getHighestRowAndColumn(): array
         // Lookup highest column and highest row
         $maxRow = $maxColumn = 1;
         foreach ($this->index as $coordinate) {
-            $row = (int) floor($coordinate / self::MAX_COLUMN_ID) + 1;
+            $row = (int) floor(($coordinate - 1) / self::MAX_COLUMN_ID) + 1;
             $maxRow = ($maxRow > $row) ? $maxRow : $row;
-            $column = $coordinate % self::MAX_COLUMN_ID;
+            $column = ($coordinate % self::MAX_COLUMN_ID) ?: self::MAX_COLUMN_ID;
             $maxColumn = ($maxColumn > $column) ? $maxColumn : $column;
         }
 
@@ -226,7 +238,7 @@ public function getHighestColumn($row = null)
             if ($coordinate < $fromRow || $coordinate >= $toRow) {
                 continue;
             }
-            $column = $coordinate % self::MAX_COLUMN_ID;
+            $column = ($coordinate % self::MAX_COLUMN_ID) ?: self::MAX_COLUMN_ID;
             $maxColumn = $maxColumn > $column ? $maxColumn : $column;
         }
 
diff --git a/src/PhpSpreadsheet/Worksheet/Worksheet.php b/src/PhpSpreadsheet/Worksheet/Worksheet.php
@@ -3203,6 +3203,9 @@ protected function cellToArray(Cell $cell, bool $calculateFormulas, bool $format
         return $returnValue;
     }
 
+    /** @var array<string, bool> */
+    private array $hiddenColumns;
+
     /**
      * Create array from a range of cells.
      *
@@ -3231,33 +3234,50 @@ public function rangeToArray(
         $minRow = $rangeStart[1];
         $maxCol = Coordinate::stringFromColumnIndex($rangeEnd[0]);
         $maxRow = $rangeEnd[1];
+        $minColInt = $rangeStart[0];
+        $maxColInt = $rangeEnd[0];
 
         ++$maxCol;
         $nullRow = $this->buildNullRow($nullValue, $minCol, $maxCol, $returnCellRef, $ignoreHidden);
+        $hideColumns = !empty($this->hiddenColumns);
 
+        $keys = $this->cellCollection?->getSortedCoordinatesInt() ?? [];
+        $keyIndex = 0;
+        $keysCount = count($keys);
         // Loop through rows
-        $r = -1;
         for ($row = $minRow; $row <= $maxRow; ++$row) {
-            if (($ignoreHidden === true) && ($this->getRowDimension($row)->getVisible() === false)) {
+            if (($ignoreHidden === true) && ($this->isRowVisible($row) === false)) {
                 continue;
             }
-            $rowRef = $returnCellRef ? $row : ++$r;
+            $rowRef = $returnCellRef ? $row : ($row - $minRow);
             $returnValue[$rowRef] = $nullRow;
-            $c = -1;
-            // Loop through columns in the current row
-            for ($col = $minCol; $col !== $maxCol; ++$col) {
-                if (($ignoreHidden === true) && ($this->getColumnDimension($col)->getVisible() === false)) {
-                    continue;
-                }
-                $columnRef = $returnCellRef ? $col : ++$c;
-                //    Using getCell() will create a new cell if it doesn't already exist. We don't want that to happen
-                //        so we test and retrieve directly against cellCollection
-                $cell = $this->getCellCollection()->get("{$col}{$row}");
-                if ($cell !== null) {
-                    $returnValue[$rowRef][$columnRef] = $this->cellToArray($cell, $calculateFormulas, $formatData, $nullValue);
+
+            $index = ($row - 1) * AddressRange::MAX_COLUMN_INT + 1;
+            $indexPlus = $index + AddressRange::MAX_COLUMN_INT - 1;
+            while ($keyIndex < $keysCount && $keys[$keyIndex] < $index) {
+                ++$keyIndex;
+            }
+            while ($keyIndex < $keysCount && $keys[$keyIndex] <= $indexPlus) {
+                $key = $keys[$keyIndex];
+                $thisRow = intdiv($key - 1, AddressRange::MAX_COLUMN_INT) + 1;
+                $thisCol = ($key % AddressRange::MAX_COLUMN_INT) ?: AddressRange::MAX_COLUMN_INT;
+                if ($thisCol >= $minColInt && $thisCol <= $maxColInt) {
+                    $col = Coordinate::stringFromColumnIndex($thisCol);
+                    if ($hideColumns === false || !isset($this->hiddenColumns[$col])) {
+                        $columnRef = $returnCellRef ? $col : ($thisCol - $minColInt);
+                        $cell = $this->cellCollection?->get("{$col}{$thisRow}");
+                        if ($cell !== null) {
+                            $value = $this->cellToArray($cell, $calculateFormulas, $formatData, $nullValue);
+                            if ($value !== $nullValue) {
+                                $returnValue[$rowRef][$columnRef] = $value;
+                            }
+                        }
+                    }
                 }
+                ++$keyIndex;
             }
         }
+        unset($this->hiddenColumns);
 
         // Return
         return $returnValue;
@@ -3281,14 +3301,16 @@ private function buildNullRow(
         bool $returnCellRef,
         bool $ignoreHidden,
     ): array {
+        $this->hiddenColumns = [];
         $nullRow = [];
         $c = -1;
         for ($col = $minCol; $col !== $maxCol; ++$col) {
-            if (($ignoreHidden === true) && ($this->getColumnDimension($col)->getVisible() === false)) {
-                continue;
+            if ($ignoreHidden === true && $this->columnDimensionExists($col) && $this->getColumnDimension($col)->getVisible() === false) {
+                $this->hiddenColumns[$col] = true;
+            } else {
+                $columnRef = $returnCellRef ? $col : ++$c;
+                $nullRow[$columnRef] = $nullValue;
             }
-            $columnRef = $returnCellRef ? $col : ++$c;
-            $nullRow[$columnRef] = $nullValue;
         }
 
         return $nullRow;
diff --git a/tests/PhpSpreadsheetTests/Worksheet/ToArrayTest.php b/tests/PhpSpreadsheetTests/Worksheet/ToArrayTest.php
@@ -0,0 +1,79 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpOffice\PhpSpreadsheetTests\Worksheet;
+
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PHPUnit\Framework\TestCase;
+
+class ToArrayTest extends TestCase
+{
+    public static function testToArray(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $inputArray = [
+            ['a', 'b', null, 'd', 'e'],
+            [],
+            [null, null, null, null, 'f'],
+            [],
+            [1, 0, null, 4],
+        ];
+
+        $sheet->fromArray($inputArray, null, 'A1', true);
+
+        $expected1 = [
+            ['a', 'b', null, 'd', 'e'],
+            [null, null, null, null, null],
+            [null, null, null, null, 'f'],
+            [null, null, null, null, null],
+            [1, 0, null, 4, null],
+        ];
+        $result1 = $sheet->toArray(null, false, false, false, false);
+        self::assertSame($expected1, $result1);
+
+        $expected2 = [
+            1 => ['A' => 'a', 'B' => 'b', 'C' => null, 'D' => 'd', 'E' => 'e'],
+            2 => ['A' => null, 'B' => null, 'C' => null, 'D' => null, 'E' => null],
+            3 => ['A' => null, 'B' => null, 'C' => null, 'D' => null, 'E' => 'f'],
+            4 => ['A' => null, 'B' => null, 'C' => null, 'D' => null, 'E' => null],
+            5 => ['A' => 1, 'B' => 0, 'C' => null, 'D' => 4, 'E' => null],
+        ];
+        $result2 = $sheet->toArray(null, false, false, true, false);
+        self::assertSame($expected2, $result2);
+
+        $expected3 = [
+            [null, null, null, null],
+            [null, null, null, 'f'],
+            [null, null, null, null],
+            [0, null, 4, null],
+        ];
+        $result3 = $sheet->rangeToArray('B2:E5', null, false, false, false, false);
+        self::assertSame($expected3, $result3);
+
+        $expected4 = [
+            2 => ['B' => null, 'C' => null, 'D' => null, 'E' => null],
+            3 => ['B' => null, 'C' => null, 'D' => null, 'E' => 'f'],
+            4 => ['B' => null, 'C' => null, 'D' => null, 'E' => null],
+            5 => ['B' => 0, 'C' => null, 'D' => 4, 'E' => null],
+        ];
+        $result4 = $sheet->rangeToArray('B2:E5', null, false, false, true, false);
+        self::assertSame($expected4, $result4);
+
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public static function testMaxCol(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $sheet->getCell('A1')->setValue('start');
+        $sheet->getCell('XFD1')->setValue('end');
+        $array = $sheet->toArray(null, false, false, false, false);
+        self::assertCount(1, $array);
+        self::assertCount(16384, $array[0]);
+        self::assertSame('start', $array[0][0]);
+        self::assertSame('end', $array[0][16383]);
+    }
+}
