From f2a7f9d84f628cb50e1b0dc302f10d2ba945b0ea Mon Sep 17 00:00:00 2001 From: Markus Podar Date: Mon, 3 Jan 2022 21:47:54 +0100 Subject: [PATCH] Improve performance for supporting Laravel 8.77+ cast attributes (#1292) #1289 calls `getReturnType` for every method on every model, which in turn calls `getReturnTypeFromDocBlock` which has this code: ```php $phpDocContext = (new ContextFactory())->createFromReflector($reflection); ``` Extracting the docblock is super slow, always has been. Now that we do this for every method, this adds up a lot. Performance on a private commercial project _before_ #1289 was introduced: ``` $ time ./artisan ide-helper:models --write --reset >/dev/null real 0m2.857s user 0m1.835s sys 0m0.129s ``` After #1289 : ``` $ time ./artisan ide-helper:models --write --reset >/dev/null real 0m54.147s user 0m47.132s sys 0m1.047s ``` However, in this case we **do not need** the phpdoc fallback (which is legitimate and by design for many other cases), because also the Laravel implementation only works by inspecting the _actual type_, see https://github.com/laravel/framework/blob/e0c2620b57be6416820ea7ca8e46fd2f71d2fe35/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php#L570-L575 ```php $returnType = (new ReflectionMethod($this, $method))->getReturnType(); return static::$attributeMutatorCache[get_class($this)][$key] = $returnType && $returnType instanceof ReflectionNamedType && $returnType->getName() === Attribute::class && is_callable($this->{$method}()->get); ``` This side-stepping the phpdoc parsing a) still works correctly and b) brings us back to the previous performance characteristics: ``` time ./artisan ide-helper:models --write --reset >/dev/null real 0m2.987s user 0m1.915s sys 0m0.120s ``` --- src/Console/ModelsCommand.php | 2 +- .../Attributes/Models/Simple.php | 20 +++++++++++++++++++ .../__snapshots__/Test__test__1.php | 20 +++++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/Console/ModelsCommand.php b/src/Console/ModelsCommand.php index d7ad3f0..578991c 100644 --- a/src/Console/ModelsCommand.php +++ b/src/Console/ModelsCommand.php @@ -562,7 +562,7 @@ class ModelsCommand extends Command sort($methods); foreach ($methods as $method) { $reflection = new \ReflectionMethod($model, $method); - $type = $this->getReturnType($reflection); + $type = $this->getReturnTypeFromReflection($reflection); $isAttribute = is_a($type, '\Illuminate\Database\Eloquent\Casts\Attribute', true); if ( Str::startsWith($method, 'get') && Str::endsWith( diff --git a/tests/Console/ModelsCommand/Attributes/Models/Simple.php b/tests/Console/ModelsCommand/Attributes/Models/Simple.php index 6457000..0d1eff8 100644 --- a/tests/Console/ModelsCommand/Attributes/Models/Simple.php +++ b/tests/Console/ModelsCommand/Attributes/Models/Simple.php @@ -20,4 +20,24 @@ class Simple extends Model } ); } + + /** + * ide-helper does not recognize this method being an Attribute + * because the method has no actual return type; + * phpdoc is ignored here deliberately due to performance reasons and also + * isn't supported by Laravel itself. + * + * @return \Illuminate\Database\Eloquent\Casts\Attribute + */ + public function notAnAttribute() + { + return new Attribute( + function (?string $value): ?string { + return $value; + }, + function (?string $value): ?string { + return $value; + } + ); + } } diff --git a/tests/Console/ModelsCommand/Attributes/__snapshots__/Test__test__1.php b/tests/Console/ModelsCommand/Attributes/__snapshots__/Test__test__1.php index 7e8fcd4..eabeffc 100644 --- a/tests/Console/ModelsCommand/Attributes/__snapshots__/Test__test__1.php +++ b/tests/Console/ModelsCommand/Attributes/__snapshots__/Test__test__1.php @@ -31,4 +31,24 @@ class Simple extends Model } ); } + + /** + * ide-helper does not recognize this method being an Attribute + * because the method has no actual return type; + * phpdoc is ignored here deliberately due to performance reasons and also + * isn't supported by Laravel itself. + * + * @return \Illuminate\Database\Eloquent\Casts\Attribute + */ + public function notAnAttribute() + { + return new Attribute( + function (?string $value): ?string { + return $value; + }, + function (?string $value): ?string { + return $value; + } + ); + } }