From 72a45e498748fc2618db87430e0d152df59abd01 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 22 Mar 2024 15:25:19 +0100 Subject: [PATCH 1/8] Improve ReflectionEnum->getCases() performance --- src/Reflection/Adapter/ReflectionEnum.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Reflection/Adapter/ReflectionEnum.php b/src/Reflection/Adapter/ReflectionEnum.php index c02a88555..4a3882eb1 100644 --- a/src/Reflection/Adapter/ReflectionEnum.php +++ b/src/Reflection/Adapter/ReflectionEnum.php @@ -530,14 +530,18 @@ public function getCase(string $name): ReflectionEnumUnitCase|ReflectionEnumBack /** @return list */ public function getCases(): array { - /** @psalm-suppress ImpureFunctionCall */ - return array_map(function (BetterReflectionEnumCase $case): ReflectionEnumUnitCase|ReflectionEnumBackedCase { + $cases = array_values($this->betterReflectionEnum->getCases()); + + $mappedCases = []; + foreach ($cases as $case) { if ($this->betterReflectionEnum->isBacked()) { - return new ReflectionEnumBackedCase($case); + $mappedCases[] = new ReflectionEnumBackedCase($case); + } else { + $mappedCases[] = new ReflectionEnumUnitCase($case); } + } - return new ReflectionEnumUnitCase($case); - }, array_values($this->betterReflectionEnum->getCases())); + return $mappedCases; } public function isBacked(): bool From 7aa805406e3ce8283d3dfe519092f14cfe47525c Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 22 Mar 2024 15:43:36 +0100 Subject: [PATCH 2/8] Remove unnecessary array_values() --- src/Reflection/Adapter/ReflectionEnum.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Reflection/Adapter/ReflectionEnum.php b/src/Reflection/Adapter/ReflectionEnum.php index 4a3882eb1..c0afb0f72 100644 --- a/src/Reflection/Adapter/ReflectionEnum.php +++ b/src/Reflection/Adapter/ReflectionEnum.php @@ -530,7 +530,7 @@ public function getCase(string $name): ReflectionEnumUnitCase|ReflectionEnumBack /** @return list */ public function getCases(): array { - $cases = array_values($this->betterReflectionEnum->getCases()); + $cases = $this->betterReflectionEnum->getCases(); $mappedCases = []; foreach ($cases as $case) { From a7ff65c7a43bc976b526f90676965239d6c307c9 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 22 Mar 2024 16:01:27 +0100 Subject: [PATCH 3/8] memoize cases in ReflectionEnum --- src/Reflection/Adapter/ReflectionEnum.php | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/Reflection/Adapter/ReflectionEnum.php b/src/Reflection/Adapter/ReflectionEnum.php index c0afb0f72..57d271d7d 100644 --- a/src/Reflection/Adapter/ReflectionEnum.php +++ b/src/Reflection/Adapter/ReflectionEnum.php @@ -33,6 +33,9 @@ */ final class ReflectionEnum extends CoreReflectionEnum { + /** @var list|list|null */ + private array|null $cases = null; + public function __construct(private BetterReflectionEnum $betterReflectionEnum) { unset($this->name); @@ -527,9 +530,13 @@ public function getCase(string $name): ReflectionEnumUnitCase|ReflectionEnumBack return new ReflectionEnumUnitCase($case); } - /** @return list */ + /** @return list|list */ public function getCases(): array { + if ($this->cases !== null) { + return $this->cases; + } + $cases = $this->betterReflectionEnum->getCases(); $mappedCases = []; @@ -541,7 +548,7 @@ public function getCases(): array } } - return $mappedCases; + return $this->cases = $mappedCases; } public function isBacked(): bool From 7948bc2384c1552ab12cd724b96e77095ed384e3 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 22 Mar 2024 16:02:29 +0100 Subject: [PATCH 4/8] pull out repeated isBacked() of the loop --- src/Reflection/Adapter/ReflectionEnum.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Reflection/Adapter/ReflectionEnum.php b/src/Reflection/Adapter/ReflectionEnum.php index 57d271d7d..a420fca0c 100644 --- a/src/Reflection/Adapter/ReflectionEnum.php +++ b/src/Reflection/Adapter/ReflectionEnum.php @@ -537,11 +537,12 @@ public function getCases(): array return $this->cases; } - $cases = $this->betterReflectionEnum->getCases(); + $isBacked = $this->betterReflectionEnum->isBacked(); + $cases = $this->betterReflectionEnum->getCases(); $mappedCases = []; foreach ($cases as $case) { - if ($this->betterReflectionEnum->isBacked()) { + if ($isBacked) { $mappedCases[] = new ReflectionEnumBackedCase($case); } else { $mappedCases[] = new ReflectionEnumUnitCase($case); From 12a578975905b2f3824487deb078321425c0f388 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 22 Mar 2024 16:29:11 +0100 Subject: [PATCH 5/8] Implement Memoize --- src/Reflection/Adapter/ReflectionEnum.php | 38 +++++++++++------------ src/Util/Memoize.php | 35 +++++++++++++++++++++ 2 files changed, 54 insertions(+), 19 deletions(-) create mode 100644 src/Util/Memoize.php diff --git a/src/Reflection/Adapter/ReflectionEnum.php b/src/Reflection/Adapter/ReflectionEnum.php index a420fca0c..2f7ec46a2 100644 --- a/src/Reflection/Adapter/ReflectionEnum.php +++ b/src/Reflection/Adapter/ReflectionEnum.php @@ -18,6 +18,7 @@ use Roave\BetterReflection\Reflection\ReflectionMethod as BetterReflectionMethod; use Roave\BetterReflection\Reflection\ReflectionProperty as BetterReflectionProperty; use Roave\BetterReflection\Util\FileHelper; +use Roave\BetterReflection\Util\Memoize; use ValueError; use function array_combine; @@ -33,12 +34,27 @@ */ final class ReflectionEnum extends CoreReflectionEnum { - /** @var list|list|null */ - private array|null $cases = null; + /** @var Memoize|list> */ + private Memoize $cases; public function __construct(private BetterReflectionEnum $betterReflectionEnum) { unset($this->name); + + $this->cases = new Memoize(function() { + $isBacked = $this->betterReflectionEnum->isBacked(); + $cases = $this->betterReflectionEnum->getCases(); + + $mappedCases = []; + foreach ($cases as $case) { + if ($isBacked) { + $mappedCases[] = new ReflectionEnumBackedCase($case); + } else { + $mappedCases[] = new ReflectionEnumUnitCase($case); + } + } + return $mappedCases; + }); } /** @return non-empty-string */ @@ -533,23 +549,7 @@ public function getCase(string $name): ReflectionEnumUnitCase|ReflectionEnumBack /** @return list|list */ public function getCases(): array { - if ($this->cases !== null) { - return $this->cases; - } - - $isBacked = $this->betterReflectionEnum->isBacked(); - $cases = $this->betterReflectionEnum->getCases(); - - $mappedCases = []; - foreach ($cases as $case) { - if ($isBacked) { - $mappedCases[] = new ReflectionEnumBackedCase($case); - } else { - $mappedCases[] = new ReflectionEnumUnitCase($case); - } - } - - return $this->cases = $mappedCases; + return $this->cases->memoize(); } public function isBacked(): bool diff --git a/src/Util/Memoize.php b/src/Util/Memoize.php new file mode 100644 index 000000000..c39cb3942 --- /dev/null +++ b/src/Util/Memoize.php @@ -0,0 +1,35 @@ +fn = $fn; + } + + /** + * @return T + */ + public function memoize(): mixed { + if ($this->cached === null) { + $this->cached = ($this->fn)(); + } + return $this->cached; + } +} From cf29bb03e4b8dcbc41975e54211a6e89e29459a3 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 22 Mar 2024 16:33:41 +0100 Subject: [PATCH 6/8] Discard function after use --- src/Util/Memoize.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Util/Memoize.php b/src/Util/Memoize.php index c39cb3942..1b3e24d7a 100644 --- a/src/Util/Memoize.php +++ b/src/Util/Memoize.php @@ -13,9 +13,9 @@ final class Memoize { private mixed $cached = null; /** - * @var callable(): T + * @var (callable(): T)|null */ - private $fn; + private $fn; /** @param callable(): T $fn */ public function __construct(callable $fn) @@ -27,8 +27,9 @@ public function __construct(callable $fn) * @return T */ public function memoize(): mixed { - if ($this->cached === null) { + if ($this->cached === null && $this->fn !== null) { $this->cached = ($this->fn)(); + $this->fn = null; } return $this->cached; } From 646e4e130a0dcaf03ac26a7205c79aebcfeb412a Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 22 Mar 2024 23:09:16 +0100 Subject: [PATCH 7/8] incorporate feedback --- src/Reflection/Adapter/ReflectionEnum.php | 5 ++-- src/Util/Memoize.php | 34 +++++++++-------------- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/src/Reflection/Adapter/ReflectionEnum.php b/src/Reflection/Adapter/ReflectionEnum.php index 2f7ec46a2..131aa5588 100644 --- a/src/Reflection/Adapter/ReflectionEnum.php +++ b/src/Reflection/Adapter/ReflectionEnum.php @@ -41,7 +41,7 @@ public function __construct(private BetterReflectionEnum $betterReflectionEnum) { unset($this->name); - $this->cases = new Memoize(function() { + $this->cases = new Memoize(function () { $isBacked = $this->betterReflectionEnum->isBacked(); $cases = $this->betterReflectionEnum->getCases(); @@ -53,6 +53,7 @@ public function __construct(private BetterReflectionEnum $betterReflectionEnum) $mappedCases[] = new ReflectionEnumUnitCase($case); } } + return $mappedCases; }); } @@ -549,7 +550,7 @@ public function getCase(string $name): ReflectionEnumUnitCase|ReflectionEnumBack /** @return list|list */ public function getCases(): array { - return $this->cases->memoize(); + return $this->cases->get(); } public function isBacked(): bool diff --git a/src/Util/Memoize.php b/src/Util/Memoize.php index 1b3e24d7a..03a415f74 100644 --- a/src/Util/Memoize.php +++ b/src/Util/Memoize.php @@ -4,33 +4,25 @@ namespace Roave\BetterReflection\Util; -/** @template T */ -final class Memoize { - /** - * @var T - * @psalm-suppress PossiblyNullPropertyAssignmentValue - */ - private mixed $cached = null; +use Closure; - /** - * @var (callable(): T)|null - */ - private $fn; +/** * @template T * @internal do not touch: you have been warned. */ +final class Memoize +{ + private readonly mixed $cached; - /** @param callable(): T $fn */ - public function __construct(callable $fn) + /** @param pure-Closure(): T $cb */ + public function __construct(private Closure|null $cb) { - $this->fn = $fn; } - /** - * @return T - */ - public function memoize(): mixed { - if ($this->cached === null && $this->fn !== null) { - $this->cached = ($this->fn)(); - $this->fn = null; + public function get(): mixed + { + if ($this->cb) { + $this->cached = ($this->cb)(); + $this->cb = null; } + return $this->cached; } } From 3de5a55f153b50726384216862041275a7c79faa Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 22 Mar 2024 23:12:49 +0100 Subject: [PATCH 8/8] missing return --- src/Util/Memoize.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Util/Memoize.php b/src/Util/Memoize.php index 03a415f74..8046a489e 100644 --- a/src/Util/Memoize.php +++ b/src/Util/Memoize.php @@ -16,6 +16,7 @@ public function __construct(private Closure|null $cb) { } + /** @return T */ public function get(): mixed { if ($this->cb) {