More significant reductions in memory usage for peaks, and faster execution times for key Cell Collection methods

This commit is contained in:
MarkBaker 2022-04-30 17:07:24 +02:00
parent 7169648a64
commit b34f0a96a7
3 changed files with 66 additions and 59 deletions

View File

@ -4260,16 +4260,6 @@ parameters:
count: 1
path: src/PhpSpreadsheet/Worksheet/Worksheet.php
-
message: "#^Parameter \\#1 \\$row of method PhpOffice\\\\PhpSpreadsheet\\\\Collection\\\\Cells\\:\\:removeRow\\(\\) expects string, int given\\.$#"
count: 1
path: src/PhpSpreadsheet/Worksheet/Worksheet.php
-
message: "#^Parameter \\#1 \\$row of method PhpOffice\\\\PhpSpreadsheet\\\\Collection\\\\Cells\\:\\:removeRow\\(\\) expects string, int\\<1, max\\> given\\.$#"
count: 1
path: src/PhpSpreadsheet/Worksheet/Worksheet.php
-
message: "#^Parameter \\#2 \\$format of static method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\:\\:toFormattedString\\(\\) expects string, string\\|null given\\.$#"
count: 2

View File

@ -12,6 +12,8 @@ use Psr\SimpleCache\CacheInterface;
class Cells
{
protected const MAX_COLUMN_ID = 16384;
/**
* @var CacheInterface
*/
@ -46,9 +48,10 @@ class Cells
private $currentCellIsDirty = false;
/**
* An index of existing cells. Booleans indexed by their coordinate.
* An index of existing cells. int pointer to the coordinate (0-base-indexed row * 16,384 + 1-base indexed column)
* indexed by their coordinate.
*
* @var bool[]
* @var int[]
*/
private $index = [];
@ -150,15 +153,9 @@ class Cells
*/
public function getSortedCoordinates()
{
$sortKeys = [];
foreach ($this->getCoordinates() as $coordinate) {
sscanf($coordinate, '%[A-Z]%d', $column, $row);
$key = (--$row * 16384) + Coordinate::columnIndexFromString($column);
$sortKeys[$key] = $coordinate;
}
ksort($sortKeys);
asort($this->index);
return array_values($sortKeys);
return array_keys($this->index);
}
/**
@ -203,21 +200,17 @@ class Cells
public function getHighestRowAndColumn()
{
// Lookup highest column and highest row
$columns = ['1A'];
$rows = [1];
foreach ($this->getCoordinates() as $coordinate) {
sscanf($coordinate, '%[A-Z]%d', $column, $row);
$rows[$row] = $rows[$row] ?? $row;
$columns[$column] = $columns[$column] ?? strlen($column) . $column;
$maxRow = $maxColumn = 1;
foreach ($this->index as $coordinate) {
$row = (int) floor($coordinate / self::MAX_COLUMN_ID) + 1;
$maxRow = ($maxRow > $row) ? $maxRow : $row;
$column = $coordinate % self::MAX_COLUMN_ID;
$maxColumn = ($maxColumn > $column) ? $maxColumn : $column;
}
// Determine highest column and row
$highestRow = max($rows);
$highestColumn = substr((string) max($columns), 1);
return [
'row' => $highestRow,
'column' => $highestColumn,
'row' => $maxRow,
'column' => Coordinate::stringFromColumnIndex($maxColumn),
];
}
@ -235,17 +228,23 @@ class Cells
return $this->getHighestRowAndColumn()['column'];
}
$maxColumn = '1A';
foreach ($this->getCoordinates() as $coord) {
sscanf($coord, '%[A-Z]%d', $c, $r);
if ($r != $row) {
continue;
}
$sortableColum = strlen($c) . $c;
$maxColumn = $maxColumn > $sortableColum ? $maxColumn : $sortableColum;
$row = (int) $row;
if ($row <= 0) {
throw new PhpSpreadsheetException('Row number must be a positive integer');
}
return substr($maxColumn, 1);
$maxColumn = 1;
$toRow = $row * self::MAX_COLUMN_ID;
$fromRow = --$row * self::MAX_COLUMN_ID;
foreach ($this->index as $coordinate) {
if ($coordinate < $fromRow || $coordinate > $toRow) {
continue;
}
$column = $coordinate % self::MAX_COLUMN_ID;
$maxColumn = $maxColumn > $column ? $maxColumn : $column;
}
return Coordinate::stringFromColumnIndex($maxColumn);
}
/**
@ -263,12 +262,13 @@ class Cells
}
$maxRow = 1;
foreach ($this->getCoordinates() as $coord) {
sscanf($coord, '%[A-Z]%d', $c, $r);
if ($c != $column) {
$columnIndex = Coordinate::columnIndexFromString($column);
foreach ($this->index as $coordinate) {
if ($coordinate % self::MAX_COLUMN_ID !== $columnIndex) {
continue;
}
$maxRow = ($maxRow > $r) ? $maxRow : $r;
$row = (int) floor($coordinate / self::MAX_COLUMN_ID) + 1;
$maxRow = ($maxRow > $row) ? $maxRow : $row;
}
return $maxRow;
@ -327,14 +327,23 @@ class Cells
/**
* Remove a row, deleting all cells in that row.
*
* @param string $row Row number to remove
* @param int|string $row Row number to remove
*/
public function removeRow($row): void
{
foreach ($this->getCoordinates() as $coord) {
sscanf($coord, '%[A-Z]%d', $c, $r);
if ($r == $row) {
$this->delete($coord);
$this->storeCurrentCell();
$row = (int) $row;
if ($row <= 0) {
throw new PhpSpreadsheetException('Row number must be a positive integer');
}
$toRow = $row * self::MAX_COLUMN_ID;
$fromRow = --$row * self::MAX_COLUMN_ID;
foreach ($this->index as $coordinate) {
if ($coordinate >= $fromRow && $coordinate < $toRow) {
$row = (int) floor($coordinate / self::MAX_COLUMN_ID) + 1;
$column = Coordinate::stringFromColumnIndex($coordinate % self::MAX_COLUMN_ID);
$this->delete("{$column}{$row}");
}
}
}
@ -346,10 +355,14 @@ class Cells
*/
public function removeColumn($column): void
{
foreach ($this->getCoordinates() as $coord) {
sscanf($coord, '%[A-Z]%d', $c, $r);
if ($c == $column) {
$this->delete($coord);
$this->storeCurrentCell();
$columnIndex = Coordinate::columnIndexFromString($column);
foreach ($this->index as $coordinate) {
if ($coordinate % self::MAX_COLUMN_ID === $columnIndex) {
$row = (int) floor($coordinate / self::MAX_COLUMN_ID) + 1;
$column = Coordinate::stringFromColumnIndex($coordinate % self::MAX_COLUMN_ID);
$this->delete("{$column}{$row}");
}
}
}
@ -394,7 +407,8 @@ class Cells
if ($cellCoordinate !== $this->currentCoordinate) {
$this->storeCurrentCell();
}
$this->index[$cellCoordinate] = true;
sscanf($cellCoordinate, '%[A-Z]%d', $column, $row);
$this->index[$cellCoordinate] = (--$row * self::MAX_COLUMN_ID) + Coordinate::columnIndexFromString($column);
$this->currentCoordinate = $cellCoordinate;
$this->currentCell = $cell;

View File

@ -38,18 +38,21 @@ class CellsTest extends TestCase
$cell2 = $sheet->getCell('A1');
self::assertSame($cell2, $collection->add('A1', $cell2), 'adding a second cell should return the cell');
self::assertEquals(['B2', 'A1'], $collection->getCoordinates(), 'cell list should contains the second cell');
// Note that sorts orders the collection itself, so these cells will aread be ordered for the subsequent asserions
self::assertEquals(['A1', 'B2'], $collection->getSortedCoordinates(), 'sorted cell list contains the second cell');
// Add a third cell
$cell3 = $sheet->getCell('AA1');
self::assertSame($cell3, $collection->add('AA1', $cell3), 'adding a third cell should return the cell');
self::assertEquals(['B2', 'A1', 'AA1'], $collection->getCoordinates(), 'cell list should contains the third cell');
self::assertEquals(['A1', 'B2', 'AA1'], $collection->getCoordinates(), 'cell list should contains the third cell');
// Note that sorts orders the collection itself, so these cells will aread be ordered for the subsequent asserions
self::assertEquals(['A1', 'AA1', 'B2'], $collection->getSortedCoordinates(), 'sorted cell list contains the third cell');
// Add a fourth cell
$cell4 = $sheet->getCell('Z1');
self::assertSame($cell4, $collection->add('Z1', $cell4), 'adding a fourth cell should return the cell');
self::assertEquals(['B2', 'A1', 'AA1', 'Z1'], $collection->getCoordinates(), 'cell list should contains the fourth cell');
self::assertEquals(['A1', 'AA1', 'B2', 'Z1'], $collection->getCoordinates(), 'cell list should contains the fourth cell');
// Note that sorts orders the collection itself, so these cells will aread be ordered for the subsequent asserions
self::assertEquals(['A1', 'Z1', 'AA1', 'B2'], $collection->getSortedCoordinates(), 'sorted cell list contains the fourth cell');
// Assert collection copy
@ -65,7 +68,7 @@ class CellsTest extends TestCase
// Assert deletion
$collection->delete('B2');
self::assertFalse($collection->has('B2'), 'cell should have been deleted');
self::assertEquals(['A1', 'AA1', 'Z1'], $collection->getCoordinates(), 'cell list should still contains the A1,Z1 and A11 cells');
self::assertEquals(['A1', 'Z1', 'AA1'], $collection->getCoordinates(), 'cell list should still contains the A1,Z1 and A11 cells');
// Assert update
$cell2 = $sheet->getCell('A1');
@ -74,7 +77,7 @@ class CellsTest extends TestCase
$cell3 = $sheet->getCell('C3');
self::assertSame($cell3, $collection->update($cell3), 'should silently add non-existing C3 cell');
self::assertEquals(['A1', 'AA1', 'Z1', 'C3'], $collection->getCoordinates(), 'cell list should contains the C3 cell');
self::assertEquals(['A1', 'Z1', 'AA1', 'C3'], $collection->getCoordinates(), 'cell list should contains the C3 cell');
}
public function testCacheLastCell(): void