Skip to content

Commit 9977966

Browse files
minor symfony#61239 [OptionsResolver] Optimize splitOutsideParenthesis() - 5.9x faster (bendavies)
This PR was squashed before being merged into the 7.4 branch. Discussion ---------- [OptionsResolver] Optimize splitOutsideParenthesis() - 5.9x faster | Q | A | ------------- | --- | Branch? | 7.4 | Bug fix? | no | New feature? | no <!-- if yes, also update src/**/CHANGELOG.md --> | Deprecations? | no <!-- if yes, also update UPGRADE-*.md and src/**/CHANGELOG.md --> | Issues | Fix symfony#59354 <!-- prefix each issue number with "Fix #"; no need to create an issue if none exists, explain below --> | License | MIT This PR optimises the `splitOutsideParenthesis` method in `OptionsResolver.php`, achieving a 2.91x performance improvement. I discovered this method as a performance hotspot while benchmarking a large Symfony form with many fields. Profiling revealed that `splitOutsideParenthesis` was consuming a significant portion of the form processing time. The `splitOutsideParenthesis` method (introduced in [PR symfony#59354](symfony#59354)) is called frequently during options resolution and has several performance bottlenecks: 1. Character-by-character string concatenation creates new string objects on each iteration (particularly inefficient in PHP due to copy-on-write behavior) 2. All input strings are processed the same way, regardless of complexity - no fast path for simple types 3. Multiple conditional checks per character ## Test Methodology Here's how all performance measurements were conducted: - **Benchmark tool**: hyperfine (10 runs with 1 warmup run) - **Test iterations**: 100,000 iterations per test case - **Test data**: 16 different type patterns: - Simple types: `string`, `int`, `bool`, `array` - Union types: `string|int`, `string|int|bool`, `string|int|bool|array` - Parentheses types: `string|(int|bool)`, `(string|int)|bool` - Nested types: `string|(int|(bool|float))`, `(string|int)|(bool|float)` - Array types: `string[]`, `int[]` - Class types: `MyClass`, `\\Namespace\\Class` - Complex union: `string|int|bool|array|object|resource|callable` Each optimisation was tested in isolation to measure its individual impact, then all optimisations were combined for the final benchmark. ## Optimisations ### 1. Fast Path for Simple Types (No Pipes) Most type declarations are simple types like `string`, `int`, `MyClass`, etc. without any union types. **Implementation:** ```php if (!\str_contains($type, '|')) { return [$type]; } ``` ### 2. Fast Path for Union Types (No Parentheses) Common union types like `string|int|bool` don't need complex parsing - PHP's `explode()` is much faster. **Implementation:** ```php if (!\str_contains($type, '(') && !\str_contains($type, ')')) { return \explode('|', $type); } ``` ### 3. Eliminate String Concatenation String concatenation in loops creates memory overhead. Using `substr()` avoids creating intermediate strings. **Implementation:** ```php // Instead of: $currentPart .= $char; // Use: $parts[] = \substr($type, $start, $i - $start); ``` ### 4. Switch Statement Optimisation Eliminates Multiple conditional checks per character. **Implementation:** ```php switch ($char) { case '(': ++$parenthesisLevel; break; case ')': --$parenthesisLevel; break; case '|': // ... } ``` ## Benchmark Results ### Individual Optimisation Impact Testing each optimisation in isolation: ```bash hyperfine --warmup 1 --runs 10 \ --sort=command \ --reference 'php test_original.php' \ 'php test_opt1_fast_path_simple.php' \ 'php test_opt2_fast_path_union.php' \ 'php test_opt3_no_string_concat.php' \ 'php test_opt4_switch_statement.php' ``` ``` Relative speed comparison 1.00 php test_original.php 1.23 ± 0.02 php test_opt1_fast_path_simple.php 1.95 ± 0.04 php test_opt2_fast_path_union.php 1.13 ± 0.03 php test_opt3_no_string_concat.php 1.35 ± 0.03 php test_opt4_switch_statement.php ``` ### Combined Optimisation Impact Combining all optimisations: ```bash hyperfine --warmup 1 --runs 10 \ --sort=command \ --reference 'php test_original.php' \ 'php test_optimised.php' ``` ``` Relative speed comparison 1.00 php test_original.php 2.91 ± 0.03 php test_optimised.php ``` Commits ------- b568fef [OptionsResolver] Optimize splitOutsideParenthesis() - 5.9x faster
2 parents 3ffc9ab + b568fef commit 9977966

File tree

2 files changed

+162
-27
lines changed

2 files changed

+162
-27
lines changed

src/Symfony/Component/OptionsResolver/OptionsResolver.php

Lines changed: 23 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1215,33 +1215,29 @@ private function verifyTypes(string $type, mixed $value, ?array &$invalidTypes =
12151215
*/
12161216
private function splitOutsideParenthesis(string $type): array
12171217
{
1218-
$parts = [];
1219-
$currentPart = '';
1220-
$parenthesisLevel = 0;
1221-
1222-
$typeLength = \strlen($type);
1223-
for ($i = 0; $i < $typeLength; ++$i) {
1224-
$char = $type[$i];
1225-
1226-
if ('(' === $char) {
1227-
++$parenthesisLevel;
1228-
} elseif (')' === $char) {
1229-
--$parenthesisLevel;
1230-
}
1231-
1232-
if ('|' === $char && 0 === $parenthesisLevel) {
1233-
$parts[] = $currentPart;
1234-
$currentPart = '';
1235-
} else {
1236-
$currentPart .= $char;
1237-
}
1238-
}
1239-
1240-
if ('' !== $currentPart) {
1241-
$parts[] = $currentPart;
1242-
}
1243-
1244-
return $parts;
1218+
return preg_split(<<<'EOF'
1219+
/
1220+
# Define a recursive subroutine for matching balanced parentheses
1221+
(?(DEFINE)
1222+
(?<balanced>
1223+
\( # Match an opening parenthesis
1224+
(?: # Start a non-capturing group for the contents
1225+
[^()] # Match any character that is not a parenthesis
1226+
| # OR
1227+
(?&balanced) # Recursively match a nested balanced group
1228+
)* # Repeat the group for all contents
1229+
\) # Match the final closing parenthesis
1230+
)
1231+
)
1232+
1233+
# Match any balanced parenthetical group, then skip it
1234+
(?&balanced)(*SKIP)(*FAIL) # Use the defined subroutine and discard the match
1235+
1236+
| # OR
1237+
1238+
\| # Match the pipe delimiter (only if not inside a skipped group)
1239+
/x
1240+
EOF, $type);
12451241
}
12461242

12471243
/**

src/Symfony/Component/OptionsResolver/Tests/OptionsResolverTest.php

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2044,6 +2044,145 @@ public function testNestedArraysException()
20442044
]);
20452045
}
20462046

2047+
/**
2048+
* @dataProvider provideValidDeeplyNestedUnionTypes
2049+
*/
2050+
public function testDeeplyNestedUnionTypes(string $type, $validValue)
2051+
{
2052+
$this->resolver->setDefined('option');
2053+
$this->resolver->setAllowedTypes('option', $type);
2054+
$this->assertEquals(['option' => $validValue], $this->resolver->resolve(['option' => $validValue]));
2055+
}
2056+
2057+
/**
2058+
* @dataProvider provideInvalidDeeplyNestedUnionTypes
2059+
*/
2060+
public function testDeeplyNestedUnionTypesException(string $type, $invalidValue, string $expectedExceptionMessage)
2061+
{
2062+
$this->resolver->setDefined('option');
2063+
$this->resolver->setAllowedTypes('option', $type);
2064+
2065+
$this->expectException(InvalidOptionsException::class);
2066+
$this->expectExceptionMessage($expectedExceptionMessage);
2067+
2068+
$this->resolver->resolve(['option' => $invalidValue]);
2069+
}
2070+
2071+
public function provideValidDeeplyNestedUnionTypes(): array
2072+
{
2073+
$resource = fopen('php://memory', 'r');
2074+
$object = new \stdClass();
2075+
2076+
return [
2077+
// Test 1 level of nesting
2078+
['string|(int|bool)', 'test'],
2079+
['string|(int|bool)', 42],
2080+
['string|(int|bool)', true],
2081+
2082+
// Test 2 levels of nesting
2083+
['string|(int|(bool|float))', 'test'],
2084+
['string|(int|(bool|float))', 42],
2085+
['string|(int|(bool|float))', true],
2086+
['string|(int|(bool|float))', 3.14],
2087+
2088+
// Test 3 levels of nesting
2089+
['string|(int|(bool|(float|null)))', 'test'],
2090+
['string|(int|(bool|(float|null)))', 42],
2091+
['string|(int|(bool|(float|null)))', true],
2092+
['string|(int|(bool|(float|null)))', 3.14],
2093+
['string|(int|(bool|(float|null)))', null],
2094+
2095+
// Test 4 levels of nesting
2096+
['string|(int|(bool|(float|(null|object))))', 'test'],
2097+
['string|(int|(bool|(float|(null|object))))', 42],
2098+
['string|(int|(bool|(float|(null|object))))', true],
2099+
['string|(int|(bool|(float|(null|object))))', 3.14],
2100+
['string|(int|(bool|(float|(null|object))))', null],
2101+
['string|(int|(bool|(float|(null|object))))', $object],
2102+
2103+
// Test complex case with multiple deep nesting
2104+
['(string|(int|bool))|(float|(null|object))', 'test'],
2105+
['(string|(int|bool))|(float|(null|object))', 42],
2106+
['(string|(int|bool))|(float|(null|object))', true],
2107+
['(string|(int|bool))|(float|(null|object))', 3.14],
2108+
['(string|(int|bool))|(float|(null|object))', null],
2109+
['(string|(int|bool))|(float|(null|object))', $object],
2110+
2111+
// Test nested at the beginning
2112+
['((string|int)|bool)|float', 'test'],
2113+
['((string|int)|bool)|float', 42],
2114+
['((string|int)|bool)|float', true],
2115+
['((string|int)|bool)|float', 3.14],
2116+
2117+
// Test multiple unions at different levels
2118+
['string|(int|(bool|float))|null|(object|(array|resource))', 'test'],
2119+
['string|(int|(bool|float))|null|(object|(array|resource))', 42],
2120+
['string|(int|(bool|float))|null|(object|(array|resource))', true],
2121+
['string|(int|(bool|float))|null|(object|(array|resource))', 3.14],
2122+
['string|(int|(bool|float))|null|(object|(array|resource))', null],
2123+
['string|(int|(bool|float))|null|(object|(array|resource))', $object],
2124+
['string|(int|(bool|float))|null|(object|(array|resource))', []],
2125+
['string|(int|(bool|float))|null|(object|(array|resource))', $resource],
2126+
2127+
// Test arrays with nested union types:
2128+
['(string|int)[]|(bool|float)[]', ['test', 42]],
2129+
['(string|int)[]|(bool|float)[]', [true, 3.14]],
2130+
2131+
// Test deeply nested arrays with unions
2132+
['((string|int)|(bool|float))[]', ['test', 42, true, 3.14]],
2133+
2134+
// Test complex nested array types
2135+
['(string|(int|bool)[])|(float|(null|object)[])', 'test'],
2136+
['(string|(int|bool)[])|(float|(null|object)[])', [42, true]],
2137+
['(string|(int|bool)[])|(float|(null|object)[])', 3.14],
2138+
['(string|(int|bool)[])|(float|(null|object)[])', [null, $object]],
2139+
2140+
// Test multi-dimensional arrays with nesting
2141+
['((string|int)[]|(bool|float)[])|null', ['test', 42]],
2142+
['((string|int)[]|(bool|float)[])|null', [true, 3.14]],
2143+
['((string|int)[]|(bool|float)[])|null', null],
2144+
];
2145+
}
2146+
2147+
public function provideInvalidDeeplyNestedUnionTypes(): array
2148+
{
2149+
$resource = fopen('php://memory', 'r');
2150+
$object = new \stdClass();
2151+
2152+
return [
2153+
// Test 1 level of nesting
2154+
['string|(int|bool)', [], 'The option "option" with value array is expected to be of type "string|(int|bool)", but is of type "array".'],
2155+
['string|(int|bool)', $object, 'The option "option" with value stdClass is expected to be of type "string|(int|bool)", but is of type "stdClass".'],
2156+
['string|(int|bool)', $resource, 'The option "option" with value resource is expected to be of type "string|(int|bool)", but is of type "resource (stream)".'],
2157+
['string|(int|bool)', null, 'The option "option" with value null is expected to be of type "string|(int|bool)", but is of type "null".'],
2158+
['string|(int|bool)', 3.14, 'The option "option" with value 3.14 is expected to be of type "string|(int|bool)", but is of type "float".'],
2159+
2160+
// Test 2 levels of nesting
2161+
['string|(int|(bool|float))', [], 'The option "option" with value array is expected to be of type "string|(int|(bool|float))", but is of type "array".'],
2162+
['string|(int|(bool|float))', $object, 'The option "option" with value stdClass is expected to be of type "string|(int|(bool|float))", but is of type "stdClass".'],
2163+
['string|(int|(bool|float))', $resource, 'The option "option" with value resource is expected to be of type "string|(int|(bool|float))", but is of type "resource (stream)".'],
2164+
['string|(int|(bool|float))', null, 'The option "option" with value null is expected to be of type "string|(int|(bool|float))", but is of type "null".'],
2165+
2166+
// Test 3 levels of nesting
2167+
['string|(int|(bool|(float|null)))', [], 'The option "option" with value array is expected to be of type "string|(int|(bool|(float|null)))", but is of type "array".'],
2168+
['string|(int|(bool|(float|null)))', $object, 'The option "option" with value stdClass is expected to be of type "string|(int|(bool|(float|null)))", but is of type "stdClass".'],
2169+
['string|(int|(bool|(float|null)))', $resource, 'The option "option" with value resource is expected to be of type "string|(int|(bool|(float|null)))", but is of type "resource (stream)".'],
2170+
2171+
// Test arrays with nested union types
2172+
['(string|int)[]|(bool|float)[]', ['test', true], 'The option "option" with value array is expected to be of type "(string|int)[]|(bool|float)[]", but one of the elements is of type "array".'],
2173+
['(string|int)[]|(bool|float)[]', [42, 3.14], 'The option "option" with value array is expected to be of type "(string|int)[]|(bool|float)[]", but one of the elements is of type "array".'],
2174+
2175+
// Test deeply nested arrays with unions
2176+
['((string|int)|(bool|float))[]', 'test', 'The option "option" with value "test" is expected to be of type "((string|int)|(bool|float))[]", but is of type "string".'],
2177+
['((string|int)|(bool|float))[]', [null], 'The option "option" with value array is expected to be of type "((string|int)|(bool|float))[]", but one of the elements is of type "null".'],
2178+
['((string|int)|(bool|float))[]', [$object], 'The option "option" with value array is expected to be of type "((string|int)|(bool|float))[]", but one of the elements is of type "stdClass".'],
2179+
2180+
// Test complex nested array types
2181+
['(string|(int|bool)[])|(float|(null|object)[])', ['test'], 'The option "option" with value array is expected to be of type "(string|(int|bool)[])|(float|(null|object)[])", but is of type "array".'],
2182+
['(string|(int|bool)[])|(float|(null|object)[])', [3.14], 'The option "option" with value array is expected to be of type "(string|(int|bool)[])|(float|(null|object)[])", but is of type "array".'],
2183+
];
2184+
}
2185+
20472186
public function testNestedArrayException1()
20482187
{
20492188
$this->expectException(InvalidOptionsException::class);

0 commit comments

Comments
 (0)