Skip to content

Commit 0254213

Browse files
committed
Remove large array type-specific performance optimisation
Fixes #441. The bug in #441 was caused by refactoring of the optimisation which introduced a type-checking error. Noting the performance impact is negligible for all cases other than large arrays of strings or numbers, and introduces significant cognitive complexity to a project that is extremely short of maintainers, removing it seems like the best course of action. The performance improvement provided by this optimisation was approximately 40%, however it also carried a number of other problematic bugs - if it were to be reintroduced at a later date with those bugs fixed (mainly the skipping of much of the validation logic for optimised items, even in cases where that logic might be necessary), it would not have such a significant impact.
1 parent dd084db commit 0254213

File tree

1 file changed

+15
-41
lines changed

1 file changed

+15
-41
lines changed

src/JsonSchema/Constraints/CollectionConstraint.php

Lines changed: 15 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -66,53 +66,27 @@ protected function validateItems(&$value, $schema = null, JsonPointer $path = nu
6666
{
6767
if (is_object($schema->items)) {
6868
// just one type definition for the whole array
69+
foreach ($value as $k => &$v) {
70+
$initErrors = $this->getErrors();
6971

70-
if (isset($schema->items->type)
71-
&& (
72-
$schema->items->type == 'string'
73-
|| $schema->items->type == 'number'
74-
|| $schema->items->type == 'integer'
75-
)
76-
&& !isset($schema->additionalItems)
77-
) {
78-
// performance optimization
79-
$type = $schema->items->type;
80-
$typeValidator = $this->factory->createInstanceFor('type');
81-
$validator = $this->factory->createInstanceFor($type === 'integer' ? 'number' : $type);
82-
83-
foreach ($value as $k => &$v) {
84-
$k_path = $this->incrementPath($path, $k);
85-
$typeValidator->check($v, $schema->items, $k_path, $i);
72+
// First check if its defined in "items"
73+
$this->checkUndefined($v, $schema->items, $path, $k);
8674

87-
$validator->check($v, $schema->items, $k_path, $i);
75+
// Recheck with "additionalItems" if the first test fails
76+
if (count($initErrors) < count($this->getErrors()) && (isset($schema->additionalItems) && $schema->additionalItems !== false)) {
77+
$secondErrors = $this->getErrors();
78+
$this->checkUndefined($v, $schema->additionalItems, $path, $k);
8879
}
89-
unset($v); /* remove dangling reference to prevent any future bugs
90-
* caused by accidentally using $v elsewhere */
91-
$this->addErrors($typeValidator->getErrors());
92-
$this->addErrors($validator->getErrors());
93-
} else {
94-
foreach ($value as $k => &$v) {
95-
$initErrors = $this->getErrors();
96-
97-
// First check if its defined in "items"
98-
$this->checkUndefined($v, $schema->items, $path, $k);
99-
100-
// Recheck with "additionalItems" if the first test fails
101-
if (count($initErrors) < count($this->getErrors()) && (isset($schema->additionalItems) && $schema->additionalItems !== false)) {
102-
$secondErrors = $this->getErrors();
103-
$this->checkUndefined($v, $schema->additionalItems, $path, $k);
104-
}
10580

106-
// Reset errors if needed
107-
if (isset($secondErrors) && count($secondErrors) < count($this->getErrors())) {
108-
$this->errors = $secondErrors;
109-
} elseif (isset($secondErrors) && count($secondErrors) === count($this->getErrors())) {
110-
$this->errors = $initErrors;
111-
}
81+
// Reset errors if needed
82+
if (isset($secondErrors) && count($secondErrors) < count($this->getErrors())) {
83+
$this->errors = $secondErrors;
84+
} elseif (isset($secondErrors) && count($secondErrors) === count($this->getErrors())) {
85+
$this->errors = $initErrors;
11286
}
113-
unset($v); /* remove dangling reference to prevent any future bugs
114-
* caused by accidentally using $v elsewhere */
11587
}
88+
unset($v); /* remove dangling reference to prevent any future bugs
89+
* caused by accidentally using $v elsewhere */
11690
} else {
11791
// Defined item type definitions
11892
foreach ($value as $k => &$v) {

0 commit comments

Comments
 (0)