From 1bb923d452f6a3c78f7171249a8d1fa146fc08e8 Mon Sep 17 00:00:00 2001 From: Baptiste Langlade Date: Sat, 9 Nov 2024 14:59:49 +0100 Subject: [PATCH 1/5] remove the need to store generator keys --- src/Accumulate.php | 24 ++++++------------------ src/Sequence/Defer.php | 8 +++++--- src/Sequence/Implementation.php | 2 +- src/Sequence/Lazy.php | 4 ++-- src/Sequence/Primitive.php | 2 +- src/Set/Defer.php | 2 +- src/Set/Implementation.php | 2 +- src/Set/Lazy.php | 2 +- src/Set/Primitive.php | 2 +- 9 files changed, 19 insertions(+), 29 deletions(-) diff --git a/src/Accumulate.php b/src/Accumulate.php index 829b3a6b..624c58b6 100644 --- a/src/Accumulate.php +++ b/src/Accumulate.php @@ -7,23 +7,20 @@ * Simple iterator to cache the results of a generator so it can be iterated * over multiple times * - * @template T * @template S - * @implements \Iterator + * @implements \Iterator * @internal Do not use this in your code * @psalm-immutable Not really immutable but to simplify declaring immutability of other structures */ final class Accumulate implements \Iterator { - /** @var \Generator */ + /** @var \Generator */ private \Generator $generator; - /** @var list */ - private array $keys = []; /** @var list */ private array $values = []; /** - * @param \Generator $generator + * @param \Generator $generator */ public function __construct(\Generator $generator) { @@ -42,20 +39,18 @@ public function current(): mixed } /** - * @return T + * @return int<0, max>|null */ - public function key(): mixed + public function key(): ?int { /** @psalm-suppress UnusedMethodCall */ $this->pop(); - return \current($this->keys); + return \key($this->values); } public function next(): void { - /** @psalm-suppress InaccessibleProperty */ - \next($this->keys); /** @psalm-suppress InaccessibleProperty */ \next($this->values); @@ -67,8 +62,6 @@ public function next(): void public function rewind(): void { - /** @psalm-suppress InaccessibleProperty */ - \reset($this->keys); /** @psalm-suppress InaccessibleProperty */ \reset($this->values); } @@ -96,11 +89,6 @@ private function reachedCacheEnd(): bool private function pop(): void { if ($this->reachedCacheEnd()) { - /** - * @psalm-suppress InaccessibleProperty - * @psalm-suppress ImpureMethodCall - */ - $this->keys[] = $this->generator->key(); /** * @psalm-suppress InaccessibleProperty * @psalm-suppress ImpureMethodCall diff --git a/src/Sequence/Defer.php b/src/Sequence/Defer.php index c9c32071..fb1ad3a2 100644 --- a/src/Sequence/Defer.php +++ b/src/Sequence/Defer.php @@ -20,14 +20,16 @@ */ final class Defer implements Implementation { - /** @var \Iterator */ + /** @var \Iterator */ private \Iterator $values; + /** + * @param \Generator $generator + */ public function __construct(\Generator $generator) { /** * @psalm-suppress ImpureFunctionCall - * @var \Iterator */ $this->values = new Accumulate((static function(\Generator $generator): \Generator { /** @var T $value */ @@ -68,7 +70,7 @@ public function count(): int } /** - * @return \Iterator + * @return \Iterator */ public function iterator(): \Iterator { diff --git a/src/Sequence/Implementation.php b/src/Sequence/Implementation.php index c46df4ba..54484232 100644 --- a/src/Sequence/Implementation.php +++ b/src/Sequence/Implementation.php @@ -33,7 +33,7 @@ public function __invoke($element): self; public function size(): int; /** - * @return \Iterator + * @return \Iterator */ public function iterator(): \Iterator; diff --git a/src/Sequence/Lazy.php b/src/Sequence/Lazy.php index 8b5b11fd..645a5d80 100644 --- a/src/Sequence/Lazy.php +++ b/src/Sequence/Lazy.php @@ -69,7 +69,7 @@ public function count(): int } /** - * @return \Iterator + * @return \Iterator */ public function iterator(): \Iterator { @@ -904,7 +904,7 @@ private function load(): Implementation * * @param Implementation $sequence * - * @return \Iterator + * @return \Iterator */ private static function open( Implementation $sequence, diff --git a/src/Sequence/Primitive.php b/src/Sequence/Primitive.php index 6be04f7b..a0536a76 100644 --- a/src/Sequence/Primitive.php +++ b/src/Sequence/Primitive.php @@ -54,7 +54,7 @@ public function count(): int } /** - * @return \Iterator + * @return \Iterator */ public function iterator(): \Iterator { diff --git a/src/Set/Defer.php b/src/Set/Defer.php index 45f6fd45..10e04b81 100644 --- a/src/Set/Defer.php +++ b/src/Set/Defer.php @@ -63,7 +63,7 @@ public function count(): int } /** - * @return \Iterator + * @return \Iterator */ public function iterator(): \Iterator { diff --git a/src/Set/Implementation.php b/src/Set/Implementation.php index a6d33d74..8ddf3715 100644 --- a/src/Set/Implementation.php +++ b/src/Set/Implementation.php @@ -32,7 +32,7 @@ public function __invoke($element): self; public function size(): int; /** - * @return \Iterator + * @return \Iterator */ public function iterator(): \Iterator; diff --git a/src/Set/Lazy.php b/src/Set/Lazy.php index 1f1034b7..4f8793ff 100644 --- a/src/Set/Lazy.php +++ b/src/Set/Lazy.php @@ -64,7 +64,7 @@ public function count(): int } /** - * @return \Iterator + * @return \Iterator */ public function iterator(): \Iterator { diff --git a/src/Set/Primitive.php b/src/Set/Primitive.php index 9ff2819f..9eebf2aa 100644 --- a/src/Set/Primitive.php +++ b/src/Set/Primitive.php @@ -69,7 +69,7 @@ public function count(): int } /** - * @return \Iterator + * @return \Iterator */ public function iterator(): \Iterator { From 5b44c1576ec161ddfbc5805f07640bcc0571cccf Mon Sep 17 00:00:00 2001 From: Baptiste Langlade Date: Sat, 9 Nov 2024 16:00:13 +0100 Subject: [PATCH 2/5] do not accumulate values inside deferred sequences that nobody references --- proofs/sequence.php | 42 +++++++ src/Accumulate.php | 16 +++ src/Sequence/Defer.php | 260 ++++++++++++++++++++++++++++++----------- 3 files changed, 250 insertions(+), 68 deletions(-) diff --git a/proofs/sequence.php b/proofs/sequence.php index 606515f8..0c10c09c 100644 --- a/proofs/sequence.php +++ b/proofs/sequence.php @@ -97,4 +97,46 @@ static function($assert, $string, $chunk) { ); }, ); + + yield proof( + 'Sequende::defer() holds intermediary values even when no longer used', + given( + Set\Sequence::of(Set\Type::any()), + Set\Sequence::of(Set\Type::any()), + ), + static function($assert, $prefix, $suffix) { + $initial = Sequence::defer((static function() use ($prefix, $suffix) { + foreach ($prefix as $value) { + yield $value; + } + + foreach ($suffix as $value) { + yield $value; + } + })()); + + // This does a partial read on the generator + $assert->same( + $prefix, + $initial + ->take(\count($prefix)) + ->toList(), + ); + + // The maps are only here to wrap the generator, it doesn't change + // the values + $another = $initial + ->map(static fn($value) => [$value]) + ->map(static fn($values) => $values[0]); + unset($initial); + + // If it didn't store the intermediary values the array would miss + // the prefix values due to the partial read on the initial + // generator due to the ->take()->toList() call above + $assert->same( + [...$prefix, ...$suffix], + $another->toList(), + ); + }, + ); }; diff --git a/src/Accumulate.php b/src/Accumulate.php index 624c58b6..89a650f3 100644 --- a/src/Accumulate.php +++ b/src/Accumulate.php @@ -18,6 +18,7 @@ final class Accumulate implements \Iterator private \Generator $generator; /** @var list */ private array $values = []; + private bool $started = false; /** * @param \Generator $generator @@ -32,6 +33,8 @@ public function __construct(\Generator $generator) */ public function current(): mixed { + /** @psalm-suppress InaccessibleProperty */ + $this->started = true; /** @psalm-suppress UnusedMethodCall */ $this->pop(); @@ -43,6 +46,8 @@ public function current(): mixed */ public function key(): ?int { + /** @psalm-suppress InaccessibleProperty */ + $this->started = true; /** @psalm-suppress UnusedMethodCall */ $this->pop(); @@ -51,6 +56,8 @@ public function key(): ?int public function next(): void { + /** @psalm-suppress InaccessibleProperty */ + $this->started = true; /** @psalm-suppress InaccessibleProperty */ \next($this->values); @@ -62,12 +69,16 @@ public function next(): void public function rewind(): void { + /** @psalm-suppress InaccessibleProperty */ + $this->started = true; /** @psalm-suppress InaccessibleProperty */ \reset($this->values); } public function valid(): bool { + /** @psalm-suppress InaccessibleProperty */ + $this->started = true; /** @psalm-suppress ImpureMethodCall */ $valid = !$this->reachedCacheEnd() || $this->generator->valid(); @@ -81,6 +92,11 @@ public function valid(): bool return $valid; } + public function started(): bool + { + return $this->started; + } + private function reachedCacheEnd(): bool { return \key($this->values) === null; diff --git a/src/Sequence/Defer.php b/src/Sequence/Defer.php index fb1ad3a2..c1e047bf 100644 --- a/src/Sequence/Defer.php +++ b/src/Sequence/Defer.php @@ -20,8 +20,10 @@ */ final class Defer implements Implementation { - /** @var \Iterator */ - private \Iterator $values; + /** @var Accumulate */ + private Accumulate $values; + /** @var \Generator */ + private \Generator $generator; /** * @param \Generator $generator @@ -37,6 +39,7 @@ public function __construct(\Generator $generator) yield $value; } })($generator)); + $this->generator = $generator; } /** @@ -46,16 +49,20 @@ public function __construct(\Generator $generator) */ public function __invoke($element): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, mixed $element): \Generator { + (static function(mixed $element) use ($captured): \Generator { + $values = self::detonate($captured); + /** @var T $value */ foreach ($values as $value) { yield $value; } yield $element; - })($this->values, $element), + })($element), ); } @@ -82,10 +89,12 @@ public function iterator(): \Iterator */ public function get(int $index): Maybe { - $values = $this->values; + $captured = $this->capture(); - return Maybe::defer(static function() use ($values, $index) { + return Maybe::defer(static function() use ($captured, $index) { $iteration = 0; + /** @var \Iterator */ + $values = self::detonate($captured); foreach ($values as $value) { if ($index === $iteration) { @@ -118,13 +127,16 @@ public function diff(Implementation $sequence): Implementation */ public function distinct(): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values): \Generator { + (static function() use ($captured): \Generator { /** @var list */ $uniques = []; + /** @var \Iterator */ + $values = self::detonate($captured); - /** @var T $value */ foreach ($values as $value) { if (!\in_array($value, $uniques, true)) { $uniques[] = $value; @@ -132,7 +144,7 @@ public function distinct(): Implementation yield $value; } } - })($this->values), + })(), ); } @@ -141,12 +153,15 @@ public function distinct(): Implementation */ public function drop(int $size): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, int $toDrop): \Generator { + (static function(int $toDrop) use ($captured): \Generator { $dropped = 0; + /** @var \Iterator */ + $values = self::detonate($captured); - /** @var T $value */ foreach ($values as $value) { if ($dropped < $toDrop) { ++$dropped; @@ -156,7 +171,7 @@ public function drop(int $size): Implementation yield $value; } - })($this->values, $size), + })($size), ); } @@ -187,16 +202,20 @@ public function equals(Implementation $sequence): bool */ public function filter(callable $predicate): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, callable $predicate): \Generator { - /** @var T $value */ + (static function(callable $predicate) use ($captured): \Generator { + /** @var \Iterator */ + $values = self::detonate($captured); + foreach ($values as $value) { if ($predicate($value)) { yield $value; } } - })($this->values, $predicate), + })($predicate), ); } @@ -230,9 +249,12 @@ public function groupBy(callable $discriminator): Map */ public function first(): Maybe { - $values = $this->values; + $captured = $this->capture(); + + return Maybe::defer(static function() use ($captured) { + /** @var \Iterator */ + $values = self::detonate($captured); - return Maybe::defer(static function() use ($values) { foreach ($values as $value) { return Maybe::just($value); } @@ -247,10 +269,12 @@ public function first(): Maybe */ public function last(): Maybe { - $values = $this->values; + $captured = $this->capture(); - return Maybe::defer(static function() use ($values) { + return Maybe::defer(static function() use ($captured) { $loaded = false; + /** @var \Iterator */ + $values = self::detonate($captured); foreach ($values as $value) { $loaded = true; @@ -290,10 +314,12 @@ public function contains($element): bool */ public function indexOf($element): Maybe { - $values = $this->values; + $captured = $this->capture(); - return Maybe::defer(static function() use ($values, $element) { + return Maybe::defer(static function() use ($captured, $element) { $index = 0; + /** @var \Iterator */ + $values = self::detonate($captured); foreach ($values as $value) { if ($value === $element) { @@ -316,18 +342,22 @@ public function indexOf($element): Maybe */ public function indices(): Implementation { + $captured = $this->capture(); + /** * @psalm-suppress ImpureFunctionCall * @var Implementation<0|positive-int> */ return new self( - (static function(\Iterator $values): \Generator { + (static function() use ($captured): \Generator { $index = 0; + /** @var \Iterator */ + $values = self::detonate($captured); foreach ($values as $_) { yield $index++; } - })($this->values), + })(), ); } @@ -340,14 +370,18 @@ public function indices(): Implementation */ public function map(callable $function): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, callable $map): \Generator { - /** @var T $value */ + (static function(callable $map) use ($captured): \Generator { + /** @var \Iterator */ + $values = self::detonate($captured); + foreach ($values as $value) { yield $map($value); } - })($this->values, $function), + })($function), ); } @@ -362,10 +396,14 @@ public function map(callable $function): Implementation */ public function flatMap(callable $map, callable $exfiltrate): self { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, callable $map, callable $exfiltrate): \Generator { - /** @var T $value */ + (static function(callable $map, callable $exfiltrate) use ($captured): \Generator { + /** @var \Iterator */ + $values = self::detonate($captured); + foreach ($values as $value) { /** * @var callable(T): C $map @@ -375,7 +413,7 @@ public function flatMap(callable $map, callable $exfiltrate): self yield $inner; } } - })($this->values, $map, $exfiltrate), + })($map, $exfiltrate), ); } @@ -386,10 +424,14 @@ public function flatMap(callable $map, callable $exfiltrate): self */ public function pad(int $size, $element): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, int $toPad, mixed $element): \Generator { - /** @var T $value */ + (static function(int $toPad, mixed $element) use ($captured): \Generator { + /** @var \Iterator */ + $values = self::detonate($captured); + foreach ($values as $value) { yield $value; --$toPad; @@ -399,7 +441,7 @@ public function pad(int $size, $element): Implementation yield $element; --$toPad; } - })($this->values, $size, $element), + })($size, $element), ); } @@ -419,12 +461,15 @@ public function partition(callable $predicate): Map */ public function slice(int $from, int $until): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, int $from, int $until): \Generator { + (static function(int $from, int $until) use ($captured): \Generator { $index = 0; + /** @var \Iterator */ + $values = self::detonate($captured); - /** @var T $value */ foreach ($values as $value) { if ($index >= $from && $index < $until) { yield $value; @@ -432,7 +477,7 @@ public function slice(int $from, int $until): Implementation ++$index; } - })($this->values, $from, $until), + })($from, $until), ); } @@ -441,12 +486,15 @@ public function slice(int $from, int $until): Implementation */ public function take(int $size): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, int $size): \Generator { + (static function(int $size) use ($captured): \Generator { $taken = 0; + /** @var \Iterator */ + $values = self::detonate($captured); - /** @var T $value */ foreach ($values as $value) { if ($taken >= $size) { return; @@ -455,7 +503,7 @@ public function take(int $size): Implementation yield $value; ++$taken; } - })($this->values, $size), + })($size), ); } @@ -478,10 +526,14 @@ public function takeEnd(int $size): Implementation */ public function append(Implementation $sequence): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, Implementation $sequence): \Generator { - /** @var T $value */ + (static function(Implementation $sequence) use ($captured): \Generator { + /** @var \Iterator */ + $values = self::detonate($captured); + foreach ($values as $value) { yield $value; } @@ -490,7 +542,7 @@ public function append(Implementation $sequence): Implementation foreach ($sequence->iterator() as $value) { yield $value; } - })($this->values, $sequence), + })($sequence), ); } @@ -501,19 +553,23 @@ public function append(Implementation $sequence): Implementation */ public function prepend(Implementation $sequence): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, Implementation $sequence): \Generator { + (static function(Implementation $sequence) use ($captured): \Generator { + /** @var \Iterator */ + $values = self::detonate($captured); + /** @var T $value */ foreach ($sequence->iterator() as $value) { yield $value; } - /** @var T $value */ foreach ($values as $value) { yield $value; } - })($this->values, $sequence), + })($sequence), ); } @@ -537,14 +593,17 @@ public function intersect(Implementation $sequence): Implementation */ public function sort(callable $function): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, callable $function): \Generator { + (static function(callable $function) use ($captured): \Generator { /** @var callable(T, T): int $sorter */ $sorter = $function; $loaded = []; + /** @var \Iterator */ + $values = self::detonate($captured); - /** @var T $value */ foreach ($values as $value) { $loaded[] = $value; } @@ -554,7 +613,7 @@ public function sort(callable $function): Implementation foreach ($loaded as $value) { yield $value; } - })($this->values, $function), + })($function), ); } @@ -590,12 +649,15 @@ public function clear(): Implementation */ public function reverse(): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values): \Generator { + (static function() use ($captured): \Generator { $reversed = []; + /** @var \Iterator */ + $values = self::detonate($captured); - /** @var T $value */ foreach ($values as $value) { \array_unshift($reversed, $value); } @@ -603,7 +665,7 @@ public function reverse(): Implementation foreach ($reversed as $value) { yield $value; } - })($this->values), + })(), ); } @@ -627,14 +689,18 @@ public function toIdentity(): Identity */ public function toSequence(): Sequence { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return Sequence::defer( - (static function(\Iterator $values): \Generator { - /** @var T $value */ + (static function() use ($captured): \Generator { + /** @var \Iterator */ + $values = self::detonate($captured); + foreach ($values as $value) { yield $value; } - })($this->values), + })(), ); } @@ -643,22 +709,29 @@ public function toSequence(): Sequence */ public function toSet(): Set { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return Set::defer( - (static function(\Iterator $values): \Generator { - /** @var T $value */ + (static function() use ($captured): \Generator { + /** @var \Iterator */ + $values = self::detonate($captured); + foreach ($values as $value) { yield $value; } - })($this->values), + })(), ); } public function find(callable $predicate): Maybe { - $values = $this->values; + $captured = $this->capture(); + + return Maybe::defer(static function() use ($captured, $predicate) { + /** @var \Iterator */ + $values = self::detonate($captured); - return Maybe::defer(static function() use ($values, $predicate) { foreach ($values as $value) { /** @psalm-suppress ImpureFunctionCall */ if ($predicate($value) === true) { @@ -691,13 +764,17 @@ public function match(callable $wrap, callable $match, callable $empty) */ public function zip(Implementation $sequence): Implementation { + $captured = $this->capture(); + /** * @psalm-suppress ImpureFunctionCall * @var Implementation */ return new self( - (static function(\Iterator $self, \Iterator $other) { + (static function(\Iterator $other) use ($captured) { /** @var \Iterator $self */ + $self = self::detonate($captured); + foreach ($self as $value) { if (!$other->valid()) { return; @@ -706,7 +783,7 @@ public function zip(Implementation $sequence): Implementation yield [$value, $other->current()]; $other->next(); } - })($this->values, $sequence->iterator()), + })($sequence->iterator()), ); } @@ -719,17 +796,21 @@ public function zip(Implementation $sequence): Implementation */ public function safeguard($carry, callable $assert): self { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, mixed $carry, callable $assert): \Generator { - /** @var T $value */ + (static function(mixed $carry, callable $assert) use ($captured): \Generator { + /** @var \Iterator */ + $values = self::detonate($captured); + foreach ($values as $value) { /** @var R */ $carry = $assert($carry, $value); yield $value; } - })($this->values, $carry, $assert), + })($carry, $assert), ); } @@ -764,8 +845,13 @@ public function memoize(): Implementation */ public function dropWhile(callable $condition): self { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ - return new self((static function(\Iterator $values, callable $condition) { + return new self((static function(callable $condition) use ($captured) { + /** @var \Iterator */ + $values = self::detonate($captured); + /** @psalm-suppress ImpureMethodCall */ while ($values->valid()) { /** @@ -792,7 +878,7 @@ public function dropWhile(callable $condition): self /** @psalm-suppress ImpureMethodCall */ $values->next(); } - })($this->values, $condition)); + })($condition)); } /** @@ -802,10 +888,14 @@ public function dropWhile(callable $condition): self */ public function takeWhile(callable $condition): self { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, callable $condition): \Generator { - /** @var T $value */ + (static function(callable $condition) use ($captured): \Generator { + /** @var \Iterator */ + $values = self::detonate($captured); + foreach ($values as $value) { if (!$condition($value)) { return; @@ -813,7 +903,7 @@ public function takeWhile(callable $condition): self yield $value; } - })($this->values, $condition), + })($condition), ); } @@ -830,4 +920,38 @@ private function load(): Implementation return new Primitive($values); } + + /** + * @return array{\WeakReference>, \Iterator} + */ + private function capture(): array + { + /** @psalm-suppress ImpureMethodCall */ + return [ + \WeakReference::create($this), + match ($this->values->started()) { + true => $this->values, + false => $this->generator, + }, + ]; + } + + /** + * @template V + * + * @param array{\WeakReference>, \Iterator} $captured + * + * @return \Iterator + */ + private static function detonate(array $captured): \Iterator + { + [$ref, $generator] = $captured; + $self = $ref->get(); + + if (\is_null($self)) { + return $generator; + } + + return $self->values; + } } From db74d712c5bcd75b434cb9557af233780e190f00 Mon Sep 17 00:00:00 2001 From: Baptiste Langlade Date: Sat, 9 Nov 2024 16:17:10 +0100 Subject: [PATCH 3/5] make sure via a stack trace that values are accumulated only once when intermediary sequences are not used --- proofs/sequence.php | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/proofs/sequence.php b/proofs/sequence.php index 0c10c09c..3fdd02f8 100644 --- a/proofs/sequence.php +++ b/proofs/sequence.php @@ -139,4 +139,35 @@ static function($assert, $prefix, $suffix) { ); }, ); + + yield proof( + "Sequence::defer() stack trace doesn't show intermediary sequences when not used", + given(Set\Integers::between(1, 10)), + static function($assert, $calls) { + $expected = null; + $sequence = Sequence::defer((static function() use (&$expected) { + yield null; + + throw $expected = new Exception; + })()); + + for ($i = 0; $i < $calls; $i++) { + $sequence = $sequence->map(static fn($value) => $value); + } + + try { + $sequence->toList(); + $assert->fail('it should throw'); + } catch (Exception $e) { + $assert->same($expected, $e); + + $accumulations = \array_filter( + $e->getTrace(), + static fn($frame) => \str_ends_with($frame['file'] ?? '', 'src/Accumulate.php'), + ); + + $assert->count(1, $accumulations); + } + }, + ); }; From 1ab31960bceb677cdbebb354507af639715ff8ce Mon Sep 17 00:00:00 2001 From: Baptiste Langlade Date: Sat, 9 Nov 2024 16:19:55 +0100 Subject: [PATCH 4/5] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d21c88d0..69f13ced 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ### Changed - Use `static` closures as much as possible to reduce the probability of creating circular references by capturing `$this` as it can lead to memory root buffer exhaustion. +- Remove keeping intermediary values of a deferred `Sequence` that is referenced by no one. ### Fixed From 6b059b3e8288ff072b4aa5d97958199ff630dc86 Mon Sep 17 00:00:00 2001 From: Baptiste Langlade Date: Sat, 9 Nov 2024 16:24:28 +0100 Subject: [PATCH 5/5] move Sequence\Defer::load() to ::memoize() --- src/Sequence/Defer.php | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/src/Sequence/Defer.php b/src/Sequence/Defer.php index c1e047bf..ec4e93fc 100644 --- a/src/Sequence/Defer.php +++ b/src/Sequence/Defer.php @@ -68,7 +68,7 @@ public function __invoke($element): Implementation public function size(): int { - return $this->load()->size(); + return $this->memoize()->size(); } public function count(): int @@ -184,7 +184,7 @@ public function dropEnd(int $size): Implementation { // this cannot be optimised as the whole generator needs to be loaded // in order to know the elements to drop - return $this->load()->dropEnd($size); + return $this->memoize()->dropEnd($size); } /** @@ -192,7 +192,7 @@ public function dropEnd(int $size): Implementation */ public function equals(Implementation $sequence): bool { - return $this->load()->equals($sequence); + return $this->memoize()->equals($sequence); } /** @@ -241,7 +241,7 @@ public function foreach(callable $function): SideEffect public function groupBy(callable $discriminator): Map { /** @var Map> */ - return $this->load()->groupBy($discriminator); + return $this->memoize()->groupBy($discriminator); } /** @@ -453,7 +453,7 @@ public function pad(int $size, $element): Implementation public function partition(callable $predicate): Map { /** @var Map> */ - return $this->load()->partition($predicate); + return $this->memoize()->partition($predicate); } /** @@ -516,7 +516,7 @@ public function takeEnd(int $size): Implementation { // this cannot be optimised as the whole generator needs to be loaded // in order to know the elements to drop - return $this->load()->takeEnd($size); + return $this->memoize()->takeEnd($size); } /** @@ -835,7 +835,13 @@ public function aggregate(callable $map, callable $exfiltrate): self */ public function memoize(): Implementation { - return $this->load(); + $values = []; + + foreach ($this->values as $value) { + $values[] = $value; + } + + return new Primitive($values); } /** @@ -907,20 +913,6 @@ public function takeWhile(callable $condition): self ); } - /** - * @return Implementation - */ - private function load(): Implementation - { - $values = []; - - foreach ($this->values as $value) { - $values[] = $value; - } - - return new Primitive($values); - } - /** * @return array{\WeakReference>, \Iterator} */