Skip to content

Commit

Permalink
minor #29065 [OptionsResolver] Micro optimizations and simplification…
Browse files Browse the repository at this point in the history
…s (yceruto)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[OptionsResolver] Micro optimizations and simplifications

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #29055
| License       | MIT
| Doc PR        | -

As we know, this component has a big impact on the workflow of the `Form` component. I'm a newcomer in Blackfire, so there could be other optimizations that I can't able to see, but here we go.

For now, we've less code to maintain ![commit-changes](https://user-images.githubusercontent.com/2028198/47917083-c3619d00-de7e-11e8-826a-0c3009948d93.png) and a micro-optimizacion of performance. It'd be great if someone could try these changes in a real project to see their real value.

![orbf](https://user-images.githubusercontent.com/2028198/47915644-e2116500-de79-11e8-8648-a5e619fcd3eb.png)
![metrics](https://user-images.githubusercontent.com/2028198/47917598-61a23280-de80-11e8-9153-3ea60317f1a5.png)

https://blackfire.io/profiles/compare/a04a13d3-7f60-4434-a2b8-0762efb8fbd6/graph
https://github.com/yceruto/orbf The sample takes into account only the core extensions.

Cheers!

Commits
-------

24c2213 Optimizations and simplifications OMG Blackfire!
  • Loading branch information
nicolas-grekas committed Nov 10, 2018
2 parents 605e2de + 24c2213 commit 1b82e56
Showing 1 changed file with 22 additions and 79 deletions.
101 changes: 22 additions & 79 deletions src/Symfony/Component/OptionsResolver/OptionsResolver.php
Expand Up @@ -800,7 +800,7 @@ public function offsetGet($option/*, bool $triggerDeprecation = true*/)
$triggerDeprecation = 1 === \func_num_args() || \func_get_arg(1);

// Shortcut for resolved options
if (array_key_exists($option, $this->resolved)) {
if (isset($this->resolved[$option]) || array_key_exists($option, $this->resolved)) {
if ($triggerDeprecation && isset($this->deprecated[$option]) && (isset($this->given[$option]) || $this->calling) && \is_string($this->deprecated[$option])) {
@trigger_error(strtr($this->deprecated[$option], array('%name%' => $option)), E_USER_DEPRECATED);
}
Expand All @@ -809,7 +809,7 @@ public function offsetGet($option/*, bool $triggerDeprecation = true*/)
}

// Check whether the option is set at all
if (!array_key_exists($option, $this->defaults)) {
if (!isset($this->defaults[$option]) && !array_key_exists($option, $this->defaults)) {
if (!isset($this->defined[$option])) {
throw new NoSuchOptionException(sprintf('The option "%s" does not exist. Defined options are: "%s".', $option, implode('", "', array_keys($this->defined))));
}
Expand All @@ -827,7 +827,7 @@ public function offsetGet($option/*, bool $triggerDeprecation = true*/)
}

if (!\is_array($value)) {
throw new InvalidOptionsException(sprintf('The nested option "%s" with value %s is expected to be of type array, but is of type "%s".', $option, $this->formatValue($value), $this->formatTypeOf($value, 'array')));
throw new InvalidOptionsException(sprintf('The nested option "%s" with value %s is expected to be of type array, but is of type "%s".', $option, $this->formatValue($value), $this->formatTypeOf($value)));
}

// The following section must be protected from cyclic calls.
Expand Down Expand Up @@ -872,7 +872,7 @@ public function offsetGet($option/*, bool $triggerDeprecation = true*/)
$invalidTypes = array();

foreach ($this->allowedTypes[$option] as $type) {
$type = isset(self::$typeAliases[$type]) ? self::$typeAliases[$type] : $type;
$type = self::$typeAliases[$type] ?? $type;

if ($valid = $this->verifyTypes($type, $value, $invalidTypes)) {
break;
Expand Down Expand Up @@ -987,58 +987,33 @@ public function offsetGet($option/*, bool $triggerDeprecation = true*/)
return $value;
}

private function verifyTypes(string $type, $value, array &$invalidTypes): bool
private function verifyTypes(string $type, $value, array &$invalidTypes, int $level = 0): bool
{
if (\is_array($value) && '[]' === substr($type, -2)) {
return $this->verifyArrayType($type, $value, $invalidTypes);
}

if (self::isValueValidType($type, $value)) {
return true;
}

if (!$invalidTypes) {
$invalidTypes[$this->formatTypeOf($value, null)] = true;
}

return false;
}

private function verifyArrayType(string $type, array $value, array &$invalidTypes, int $level = 0): bool
{
$type = substr($type, 0, -2);

$suffix = '[]';
while (\strlen($suffix) <= $level * 2) {
$suffix .= '[]';
}

if ('[]' === substr($type, -2)) {
$success = true;
foreach ($value as $item) {
if (!\is_array($item)) {
$invalidTypes[$this->formatTypeOf($item, null).$suffix] = true;
$type = substr($type, 0, -2);

foreach ($value as $val) {
if (!$this->verifyTypes($type, $val, $invalidTypes, $level + 1)) {
return false;
}

if (!$this->verifyArrayType($type, $item, $invalidTypes, $level + 1)) {
$success = false;
}
}

return $success;
return true;
}

foreach ($value as $item) {
if (!self::isValueValidType($type, $item)) {
$invalidTypes[$this->formatTypeOf($item, $type).$suffix] = $value;
if (('null' === $type && null === $value) || (\function_exists($func = 'is_'.$type) && $func($value)) || $value instanceof $type) {
return true;
}

return false;
if (!$invalidTypes) {
$suffix = '';
while (\strlen($suffix) < $level * 2) {
$suffix .= '[]';
}
$invalidTypes[$this->formatTypeOf($value).$suffix] = true;
}

return true;
return false;
}

/**
Expand Down Expand Up @@ -1104,40 +1079,13 @@ public function count()
/**
* Returns a string representation of the type of the value.
*
* This method should be used if you pass the type of a value as
* message parameter to a constraint violation. Note that such
* parameters should usually not be included in messages aimed at
* non-technical people.
*
* @param mixed $value The value to return the type of
*
* @return string The type of the value
*/
private function formatTypeOf($value, ?string $type): string
private function formatTypeOf($value): string
{
$suffix = '';

if (null !== $type && '[]' === substr($type, -2)) {
$suffix = '[]';
$type = substr($type, 0, -2);
while ('[]' === substr($type, -2)) {
$type = substr($type, 0, -2);
$value = array_shift($value);
if (!\is_array($value)) {
break;
}
$suffix .= '[]';
}

if (\is_array($value)) {
$subTypes = array();
foreach ($value as $val) {
$subTypes[$this->formatTypeOf($val, null)] = true;
}

return implode('|', array_keys($subTypes)).$suffix;
}
}

return (\is_object($value) ? \get_class($value) : \gettype($value)).$suffix;
return \is_object($value) ? \get_class($value) : \gettype($value);
}

/**
Expand Down Expand Up @@ -1198,9 +1146,4 @@ private function formatValues(array $values): string

return implode(', ', $values);
}

private static function isValueValidType(string $type, $value): bool
{
return (\function_exists($isFunction = 'is_'.$type) && $isFunction($value)) || $value instanceof $type;
}
}

0 comments on commit 1b82e56

Please sign in to comment.