Skip to content

Commit bce30f9

Browse files
oleibmanoleibman
committed
Improve Xlsx Reader Speed
Backport of PR #4153.
1 parent ffbcee6 commit bce30f9

File tree

6 files changed

+126
-22
lines changed

6 files changed

+126
-22
lines changed

‎.php-cs-fixer.dist.php‎

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
$config
1111
->setRiskyAllowed(true)
1212
->setFinder($finder)
13-
->setParallelConfig(PhpCsFixer\Runner\Parallel\ParallelConfigFactory::detect())
13+
->setParallelConfig(PhpCsFixer\Runner\Parallel\ParallelConfigFactory::detect(null, 600))
1414
->setCacheFile(sys_get_temp_dir() . '/php-cs-fixer' . preg_replace('~\W~', '-', __DIR__))
1515
->setRules([
1616
'align_multiline_comment' => true,
@@ -19,18 +19,17 @@
1919
'backtick_to_shell_exec' => true,
2020
'binary_operator_spaces' => true,
2121
'blank_line_after_namespace' => true,
22+
'blank_lines_before_namespace' => ['max_line_breaks' => 2, 'min_line_breaks' => 2], // we want 1 blank line before namespace
2223
'blank_line_after_opening_tag' => true,
2324
'blank_line_before_statement' => true,
24-
'braces' => true,
2525
'cast_spaces' => true,
2626
'class_attributes_separation' => ['elements' => ['method' => 'one', 'property' => 'one']], // const are often grouped with other related const
2727
'class_definition' => false, // phpcs disagree
28-
'class_keyword_remove' => false, // Deprecated, and ::class keyword gives us better support in IDE
2928
'combine_consecutive_issets' => true,
3029
'combine_consecutive_unsets' => true,
3130
'combine_nested_dirname' => true,
3231
'comment_to_phpdoc' => false, // interferes with annotations
33-
'compact_nullable_typehint' => true,
32+
'compact_nullable_type_declaration' => true,
3433
'concat_space' => ['spacing' => 'one'],
3534
'constant_case' => true,
3635
'date_time_immutable' => false, // Break our unit tests
@@ -47,7 +46,6 @@
4746
'encoding' => true,
4847
'ereg_to_preg' => true,
4948
'error_suppression' => false, // it breaks \PhpOffice\PhpSpreadsheet\Helper\Handler
50-
'escape_implicit_backslashes' => true,
5149
'explicit_indirect_variable' => false, // I feel it makes the code actually harder to read
5250
'explicit_string_variable' => false, // I feel it makes the code actually harder to read
5351
'final_class' => false, // We need non-final classes
@@ -59,7 +57,6 @@
5957
'fully_qualified_strict_types' => true,
6058
'function_declaration' => true,
6159
'function_to_constant' => true,
62-
'function_typehint_space' => true,
6360
'general_phpdoc_annotation_remove' => ['annotations' => ['access', 'category', 'copyright']],
6461
'general_phpdoc_tag_rename' => true,
6562
'global_namespace_import' => true,
@@ -93,15 +90,13 @@
9390
'native_constant_invocation' => false, // Micro optimization that look messy
9491
'native_function_casing' => true,
9592
'native_function_invocation' => false, // I suppose this would be best, but I am still unconvinced about the visual aspect of it
96-
'native_function_type_declaration_casing' => true,
97-
'new_with_braces' => true,
93+
'new_with_parentheses' => ['anonymous_class' => true, 'named_class' => true],
9894
'no_alias_functions' => true,
9995
'no_alias_language_construct_call' => true,
10096
'no_alternative_syntax' => true,
10197
'no_binary_string' => true,
10298
'no_blank_lines_after_class_opening' => true,
10399
'no_blank_lines_after_phpdoc' => true,
104-
'no_blank_lines_before_namespace' => false, // we want 1 blank line before namespace
105100
'no_break_comment' => true,
106101
'no_closing_tag' => true,
107102
'no_empty_comment' => true,
@@ -121,16 +116,14 @@
121116
'no_space_around_double_colon' => true,
122117
'no_spaces_after_function_name' => true,
123118
'no_spaces_around_offset' => true,
124-
'no_spaces_inside_parenthesis' => true,
125119
'no_superfluous_elseif' => false, // Might be risky on a huge code base
126120
'no_superfluous_phpdoc_tags' => ['allow_mixed' => true],
127-
'no_trailing_comma_in_list_call' => true,
128-
'no_trailing_comma_in_singleline_array' => true,
121+
'no_trailing_comma_in_singleline' => ['elements' => ['arguments', 'array_destructuring', 'array', 'group_import']],
129122
'no_trailing_whitespace' => true,
130123
'no_trailing_whitespace_in_comment' => true,
131124
'no_trailing_whitespace_in_string' => false, // Too dangerous
132125
'no_unneeded_control_parentheses' => true,
133-
'no_unneeded_curly_braces' => true,
126+
'no_unneeded_braces' => true,
134127
'no_unneeded_final_method' => true,
135128
'no_unreachable_default_argument_value' => true,
136129
'no_unset_cast' => true,
@@ -175,7 +168,6 @@
175168
'phpdoc_align' => false, // Waste of time
176169
'phpdoc_annotation_without_dot' => true,
177170
'phpdoc_indent' => true,
178-
//'phpdoc_inline_tag' => true,
179171
'phpdoc_line_span' => false, // Unfortunately our old comments turn even uglier with this
180172
'phpdoc_no_access' => true,
181173
'phpdoc_no_alias_tag' => true,
@@ -215,20 +207,20 @@
215207
'simplified_if_return' => false, // Even if technically correct we prefer to be explicit
216208
'simplified_null_return' => false, // Even if technically correct we prefer to be explicit
217209
'single_blank_line_at_eof' => true,
218-
'single_blank_line_before_namespace' => true,
219210
'single_class_element_per_statement' => true,
220211
'single_import_per_statement' => true,
221212
'single_line_after_imports' => true,
222213
'single_line_comment_style' => true,
223214
'single_line_throw' => false, // I don't see any reason for having a special case for Exception
224215
'single_quote' => true,
225-
'single_space_after_construct' => true,
226216
'single_trait_insert_per_statement' => true,
227217
'space_after_semicolon' => true,
218+
'spaces_inside_parentheses' => ['space' => 'none'],
228219
'standardize_increment' => true,
229220
'standardize_not_equals' => true,
230221
'static_lambda' => false, // Risky if we can't guarantee nobody use `bindTo()`
231222
'strict_comparison' => false, // No, too dangerous to change that
223+
'string_implicit_backslashes' => false, // was escape_implicit_backslashes, too confusing
232224
'strict_param' => false, // No, too dangerous to change that
233225
'string_length_to_empty' => true,
234226
'string_line_ending' => true,
@@ -240,6 +232,7 @@
240232
'ternary_to_null_coalescing' => true,
241233
'trailing_comma_in_multiline' => true,
242234
'trim_array_spaces' => true,
235+
'type_declaration_spaces' => ['elements' => ['function', 'property']], // was function_typehint_space
243236
'types_spaces' => true,
244237
'unary_operator_spaces' => true,
245238
'use_arrow_functions' => true,

‎CHANGELOG.md‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,28 @@ All notable changes to this project will be documented in this file.
55
The format is based on [Keep a Changelog](https://keepachangelog.com)
66
and this project adheres to [Semantic Versioning](https://semver.org).
77

8+
## TBD - 2.2.3
9+
10+
### Added
11+
12+
- Nothing yet.
13+
14+
### Changed
15+
16+
- Nothing yet.
17+
18+
### Deprecated
19+
20+
- Nothing yet.
21+
22+
### Moved
23+
24+
- Nothing yet.
25+
26+
### Fixed
27+
28+
- Improve Xlsx Reader speed (backport of PR #4153 intended for 3.0.0). [Issue #3917](https://github.com/PHPOffice/PhpSpreadsheet/issues/3917)
29+
830
## 2024-08-07 - 2.2.2
931

1032
### Added

‎src/PhpSpreadsheet/Cell/Cell.php‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,21 @@ public function setValueExplicit(mixed $value, string $dataType = DataType::TYPE
316316
$this->updateInCollection();
317317
$cellCoordinate = $this->getCoordinate();
318318
self::updateIfCellIsTableHeader($this->getParent()?->getParent(), $this, $oldValue, $value);
319-
$this->getWorksheet()->applyStylesFromArray($cellCoordinate, ['quotePrefix' => $quotePrefix]);
319+
$worksheet = $this->getWorksheet();
320+
$spreadsheet = $worksheet->getParent();
321+
if (isset($spreadsheet)) {
322+
$originalSelected = $worksheet->getSelectedCells();
323+
$activeSheetIndex = $spreadsheet->getActiveSheetIndex();
324+
$style = $this->getStyle();
325+
$oldQuotePrefix = $style->getQuotePrefix();
326+
if ($oldQuotePrefix !== $quotePrefix) {
327+
$style->setQuotePrefix($quotePrefix);
328+
}
329+
$worksheet->setSelectedCells($originalSelected);
330+
if ($activeSheetIndex >= 0) {
331+
$spreadsheet->setActiveSheetIndex($activeSheetIndex);
332+
}
333+
}
320334

321335
return $this->getParent()?->get($cellCoordinate) ?? $this;
322336
}

‎src/PhpSpreadsheet/Reader/Xlsx.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -931,16 +931,16 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
931931

932932
// Style information?
933933
if (!$this->readDataOnly) {
934-
$holdSelected = $docSheet->getSelectedCells();
935934
$cAttrS = (int) ($cAttr['s'] ?? 0);
936935
// no style index means 0, it seems
937936
$cAttrS = isset($styles[$cAttrS]) ? $cAttrS : 0;
938937
$cell->setXfIndex($cAttrS);
939938
// issue 3495
940939
if ($cellDataType === DataType::TYPE_FORMULA && $styles[$cAttrS]->quotePrefix === true) {
940+
$holdSelected = $docSheet->getSelectedCells();
941941
$cell->getStyle()->setQuotePrefix(false);
942+
$docSheet->setSelectedCells($holdSelected);
942943
}
943-
$docSheet->setSelectedCells($holdSelected);
944944
}
945945
}
946946
++$rowIndex;

‎src/PhpSpreadsheet/Worksheet/Worksheet.php‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3682,10 +3682,8 @@ public function applyStylesFromArray(string $coordinate, array $styleArray): boo
36823682
}
36833683
$activeSheetIndex = $spreadsheet->getActiveSheetIndex();
36843684
$originalSelected = $this->selectedCells;
3685-
$originalActive = $this->activeCell;
36863685
$this->getStyle($coordinate)->applyFromArray($styleArray);
3687-
$this->activeCell = $originalActive;
3688-
$this->selectedCells = $originalSelected;
3686+
$this->setSelectedCells($originalSelected);
36893687
if ($activeSheetIndex >= 0) {
36903688
$spreadsheet->setActiveSheetIndex($activeSheetIndex);
36913689
}

‎tests/PhpSpreadsheetTests/Worksheet/ApplyStylesTest.php‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace PhpOffice\PhpSpreadsheetTests\Worksheet;
6+
7+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
8+
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
9+
use PHPUnit\Framework\TestCase;
10+
11+
class ApplyStylesTest extends TestCase
12+
{
13+
public function testApplyFromArray(): void
14+
{
15+
$spreadsheet = new Spreadsheet();
16+
$sheet1 = $spreadsheet->getActiveSheet();
17+
$sheet2 = $spreadsheet->createSheet();
18+
$sheet3 = $spreadsheet->createSheet();
19+
$cell = 'B4';
20+
$sheet1->getCell($cell)->setValue('first');
21+
$sheet1->getStyle($cell)->getFont()->setName('Arial');
22+
$cell = 'C9';
23+
$sheet2->getCell($cell)->setValue('second');
24+
$sheet2->getStyle($cell)->getFont()->setName('Arial');
25+
$cell = 'A6';
26+
$sheet3->getCell($cell)->setValue('third');
27+
$sheet3->getStyle($cell)->getFont()->setName('Arial');
28+
self::assertSame(2, $spreadsheet->getActiveSheetIndex());
29+
self::assertSame('B4', $sheet1->getSelectedCells());
30+
self::assertSame('C9', $sheet2->getSelectedCells());
31+
self::assertSame('A6', $sheet3->getSelectedCells());
32+
$cell = 'D12';
33+
$styleArray = ['font' => ['name' => 'Courier New']];
34+
$sheet2->getStyle($cell)->applyFromArray($styleArray);
35+
self::assertSame(1, $spreadsheet->getActiveSheetIndex());
36+
self::assertSame('B4', $sheet1->getSelectedCells());
37+
self::assertSame('D12', $sheet2->getSelectedCells());
38+
self::assertSame('A6', $sheet3->getSelectedCells());
39+
$spreadsheet->disconnectWorksheets();
40+
}
41+
42+
public function testApplyStylesFromArray(): void
43+
{
44+
$spreadsheet = new Spreadsheet();
45+
$sheet1 = $spreadsheet->getActiveSheet();
46+
$sheet2 = $spreadsheet->createSheet();
47+
$sheet3 = $spreadsheet->createSheet();
48+
$cell = 'B4';
49+
$sheet1->getCell($cell)->setValue('first');
50+
$sheet1->getStyle($cell)->getFont()->setName('Arial');
51+
$cell = 'C9';
52+
$sheet2->getCell($cell)->setValue('second');
53+
$sheet2->getStyle($cell)->getFont()->setName('Arial');
54+
$cell = 'A6';
55+
$sheet3->getCell($cell)->setValue('third');
56+
$sheet3->getStyle($cell)->getFont()->setName('Arial');
57+
self::assertSame(2, $spreadsheet->getActiveSheetIndex());
58+
self::assertSame('B4', $sheet1->getSelectedCells());
59+
self::assertSame('C9', $sheet2->getSelectedCells());
60+
self::assertSame('A6', $sheet3->getSelectedCells());
61+
$cell = 'D12';
62+
$styleArray = ['font' => ['name' => 'Courier New']];
63+
$sheet2->applyStylesFromArray($cell, $styleArray);
64+
self::assertSame(2, $spreadsheet->getActiveSheetIndex(), 'should be unchanged');
65+
self::assertSame('B4', $sheet1->getSelectedCells(), 'should be unchanged');
66+
self::assertSame('C9', $sheet2->getSelectedCells(), 'should be unchanged');
67+
self::assertSame('A6', $sheet3->getSelectedCells(), 'should be unchanged');
68+
$spreadsheet->disconnectWorksheets();
69+
}
70+
71+
public function testNoSpreadsheet(): void
72+
{
73+
$sheet2 = new Worksheet();
74+
$cell = 'D12';
75+
self::assertFalse($sheet2->applyStylesFromArray($cell, ['font' => ['name' => 'Courier New']]));
76+
}
77+
}

0 commit comments

Comments
 (0)