From 7e6db527ac52591421cb5d48b9f49623a9fd0187 Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Fri, 21 Aug 2026 14:29:56 -0400 Subject: [PATCH 1/2] perf: return the committed manifest from snapshot writes and reuse its raw rollback payload to avoid duplicate index reads and hydration. --- CHANGELOG.md | 1 + src/Storage/SnapshotStore.php | 38 +++++++++++---- src/Storage/SnapshotWriteResult.php | 20 ++++++++ tests/Storage/SnapshotStoreTest.php | 72 +++++++++++++++++++++++++++++ tests/Support/MockerExtension.php | 14 +++++- 5 files changed, 134 insertions(+), 11 deletions(-) create mode 100644 src/Storage/SnapshotWriteResult.php diff --git a/CHANGELOG.md b/CHANGELOG.md index acec6d5..0f5d2a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,3 +26,4 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - fix: harden packaging, privacy, lifecycle, snapshot recovery, dump and toolbar security, and accelerate value hydration. - refactor: simplify strict value hydration, collector cleanup reporting, sensitive-key lookup, and toolbar message validation without changing public contracts. - test: enforce complete PHP line, method, and mutation coverage with exact HTML rendering assertions. +- perf: return the committed manifest from snapshot writes and reuse its raw rollback payload to avoid duplicate index reads and hydration. diff --git a/src/Storage/SnapshotStore.php b/src/Storage/SnapshotStore.php index 58e2882..312f8fa 100644 --- a/src/Storage/SnapshotStore.php +++ b/src/Storage/SnapshotStore.php @@ -12,7 +12,6 @@ use function array_reverse; use function count; use function fclose; -use function file_get_contents; use function glob; use function is_array; use function is_dir; @@ -252,6 +251,22 @@ public function readSnapshotResult(string $tag): SnapshotReadResult * @return list Entries evicted from the manifest. */ public function writeSnapshot(DebugSnapshot $snapshot, int $historySize): array + { + return $this->writeSnapshotResult($snapshot, $historySize)->removed; + } + + /** + * Writes a snapshot and returns the already-hydrated committed manifest together with evicted entries. + * + * Adapters that need the resulting manifest can consume this result without acquiring the lock and decoding the + * index a second time. + * + * @param DebugSnapshot $snapshot Snapshot to persist. + * @param int $historySize Maximum number of retained entries. + * + * @return SnapshotWriteResult Committed manifest and entries evicted from it. + */ + public function writeSnapshotResult(DebugSnapshot $snapshot, int $historySize): SnapshotWriteResult { self::assertValidHistorySize($historySize); @@ -267,7 +282,7 @@ public function writeSnapshot(DebugSnapshot $snapshot, int $historySize): array try { $this->recoverTransaction(); - $manifest = $this->readManifestFile(); + [$manifest, $manifestBefore] = $this->readManifestFile(); $entries = ($manifest ?? $this->rebuildManifest())->entries; @@ -282,7 +297,7 @@ public function writeSnapshot(DebugSnapshot $snapshot, int $historySize): array 'state' => 'prepared', 'tag' => $tag, 'snapshotBefore' => $this->readExistingFile($snapshotFile), - 'manifestBefore' => $this->readExistingFile($this->indexFile()), + 'manifestBefore' => $manifestBefore, ]; $this->atomicWrite($this->transactionFile(), self::encode($transaction)); @@ -311,7 +326,10 @@ public function writeSnapshot(DebugSnapshot $snapshot, int $historySize): array $this->removeStaleSnapshots($entries); - return $removed; + return new SnapshotWriteResult( + array_reverse($entries, true), + $removed, + ); } finally { fclose($lock); } @@ -543,14 +561,14 @@ private function readExistingFile(string $file): string|null /** * Reads the manifest or returns `null` when persisted JSON is invalid. * - * @return Manifest|null Hydrated manifest or `null` for invalid persisted JSON. + * @return array{Manifest|null, string|null} Hydrated manifest and its raw rollback payload. */ - private function readManifestFile(): Manifest|null + private function readManifestFile(): array { $file = $this->indexFile(); if (!is_file($file)) { - return new Manifest([]); + return [new Manifest([]), null]; } $raw = @file_get_contents($file); @@ -562,13 +580,13 @@ private function readManifestFile(): Manifest|null } if ($raw === '') { - return null; + return [null, $raw]; } try { - return Manifest::fromArray(self::decode($raw)); + return [Manifest::fromArray(self::decode($raw)), $raw]; } catch (Throwable) { - return null; + return [null, $raw]; } } diff --git a/src/Storage/SnapshotWriteResult.php b/src/Storage/SnapshotWriteResult.php new file mode 100644 index 0000000..1302e40 --- /dev/null +++ b/src/Storage/SnapshotWriteResult.php @@ -0,0 +1,20 @@ + $entries Committed manifest entries, newest first. + * @param list $removed Entries evicted from the manifest. + */ + public function __construct( + public array $entries, + public array $removed, + ) {} +} diff --git a/tests/Storage/SnapshotStoreTest.php b/tests/Storage/SnapshotStoreTest.php index 9defb27..5d14966 100644 --- a/tests/Storage/SnapshotStoreTest.php +++ b/tests/Storage/SnapshotStoreTest.php @@ -8,6 +8,7 @@ use PHPUnit\Framework\Attributes\{Group, TestWith}; use PHPUnit\Framework\TestCase; use Xepozz\InternalMocker\MockerState; +use function is_array; /** * Unit tests for {@see SnapshotStore} covering the JSON filesystem boundary: atomic writes, manifest locking, history @@ -986,6 +987,77 @@ public function testSnapshotReadResultReturnsPersistedSnapshotWithoutAnError(): ); } + public function testSnapshotWriteResultReadsExistingManifestOnce(): void + { + $store = $this->store(); + + $store->writeSnapshot( + new DebugSnapshot($this->summary('older', 1_700_000_000.0), [], []), + 10, + ); + + MockerState::resetState(); + + $store->writeSnapshotResult( + new DebugSnapshot($this->summary('newer', 1_700_000_001.0), [], []), + 10, + ); + + $reads = 0; + + foreach (MockerState::getTraces('PHPForge\\Debug\\Storage', 'file_get_contents') as $trace) { + if (!is_array($trace)) { + continue; + } + + $arguments = $trace['arguments'] ?? null; + + if (is_array($arguments) && ($arguments[0] ?? null) === "{$this->path}/index.json") { + $reads++; + } + } + + self::assertSame( + 1, + $reads, + 'The existing manifest must be read once per write.', + ); + } + + public function testSnapshotWriteResultReturnsCommittedManifestAndEvictions(): void + { + $store = $this->store(); + + $older = $this->summary('older', 1_700_000_000.0); + $newer = $this->summary('newer', 1_700_000_001.0); + + $store->writeSnapshot( + new DebugSnapshot($older, [], []), + 10, + ); + + $result = $store->writeSnapshotResult( + new DebugSnapshot($newer, [], []), + 1, + ); + + self::assertSame( + ['newer'], + array_keys($result->entries), + 'Committed entries must be returned newest first.', + ); + self::assertSame( + ['older'], + array_map(static fn(RequestSummary $summary): string => $summary->tag, $result->removed), + 'Every eviction must be returned.', + ); + self::assertEquals( + $store->loadManifest(), + $result->entries, + 'Returned entries must match persisted data.', + ); + } + /** * @param string $tag Leading-dot tag. */ diff --git a/tests/Support/MockerExtension.php b/tests/Support/MockerExtension.php index 7583468..e9e88f5 100644 --- a/tests/Support/MockerExtension.php +++ b/tests/Support/MockerExtension.php @@ -68,7 +68,19 @@ public static function load(): void ]; } - foreach (['chmod', 'file_put_contents', 'flock', 'fopen', 'mkdir', 'rename', 'tempnam', 'unlink'] as $name) { + foreach ( + [ + 'chmod', + 'file_get_contents', + 'file_put_contents', + 'flock', + 'fopen', + 'mkdir', + 'rename', + 'tempnam', + 'unlink', + ] as $name + ) { $mocks[] = [ 'namespace' => 'PHPForge\Debug\Storage', 'name' => $name, From f94aa637b3df2fb559b248073f158a12b747e094 Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Fri, 21 Aug 2026 14:32:06 -0400 Subject: [PATCH 2/2] Fix ECS ci. --- tests/Storage/SnapshotStoreTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Storage/SnapshotStoreTest.php b/tests/Storage/SnapshotStoreTest.php index 5d14966..29efad6 100644 --- a/tests/Storage/SnapshotStoreTest.php +++ b/tests/Storage/SnapshotStoreTest.php @@ -8,6 +8,7 @@ use PHPUnit\Framework\Attributes\{Group, TestWith}; use PHPUnit\Framework\TestCase; use Xepozz\InternalMocker\MockerState; + use function is_array; /**