From a0e31a577ecbf65b90d1d1452cbed2c0aff0545b Mon Sep 17 00:00:00 2001 From: Daniel Leech Date: Wed, 27 May 2020 21:00:16 +0100 Subject: [PATCH 1/6] Adds phpbench --- composer.json | 3 +- lib/Model/SearchIndex.php | 2 ++ tests/Benchmark/SearchBench.php | 56 +++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 tests/Benchmark/SearchBench.php diff --git a/composer.json b/composer.json index 00d51a4..08e1438 100644 --- a/composer.json +++ b/composer.json @@ -33,7 +33,8 @@ "phpstan/phpstan": "^0.12.0", "phpactor/test-utils": "^1.0.2", "phpactor/console-extension": "^0.1.2", - "symfony/var-dumper": "^5.1" + "symfony/var-dumper": "^5.1", + "phpbench/phpbench": "^1.0@dev" }, "autoload": { "psr-4": { diff --git a/lib/Model/SearchIndex.php b/lib/Model/SearchIndex.php index 4e0ce88..b675f9a 100644 --- a/lib/Model/SearchIndex.php +++ b/lib/Model/SearchIndex.php @@ -13,5 +13,7 @@ public function search(string $query): Generator; public function write(Record $record): void; + public function remove(Record $record): void; + public function flush(): void; } diff --git a/tests/Benchmark/SearchBench.php b/tests/Benchmark/SearchBench.php new file mode 100644 index 0000000..42e8362 --- /dev/null +++ b/tests/Benchmark/SearchBench.php @@ -0,0 +1,56 @@ +search = new FileSearchIndex(__DIR__ . '/../../cache/search', new ClassShortNameMatcher()); + } + + /** + * @BeforeMethods({"setUp"}) + * @ParamProviders({"provideSearches"}) + */ + public function benchSearch(array $params): void + { + foreach ($this->search->search($params['search']) as $result) { + } + } + + public function provideSearches() + { + yield 'A' => [ + 'search' => 'A', + ]; + + yield 'B' => [ + 'search' => 'B', + ]; + + yield 'C' => [ + 'search' => 'C', + ]; + + yield 'Request' => [ + 'search' => 'Request', + ]; + } +} From fa96a2c70d08d04d5a19d26010f14729dd7555f3 Mon Sep 17 00:00:00 2001 From: Daniel Leech Date: Wed, 27 May 2020 21:14:20 +0100 Subject: [PATCH 2/6] Adds validating search --- lib/Adapter/Php/FileSearchIndex.php | 16 ++++- .../Php/InMemory/InMemorySearchIndex.php | 5 ++ .../SearchIndex/ValidatingSearchIndex.php | 70 +++++++++++++++++++ tests/Benchmark/SearchBench.php | 41 ++++++++--- 4 files changed, 120 insertions(+), 12 deletions(-) create mode 100644 lib/Model/SearchIndex/ValidatingSearchIndex.php diff --git a/lib/Adapter/Php/FileSearchIndex.php b/lib/Adapter/Php/FileSearchIndex.php index ea43bbf..95f3c06 100644 --- a/lib/Adapter/Php/FileSearchIndex.php +++ b/lib/Adapter/Php/FileSearchIndex.php @@ -66,10 +66,19 @@ public function search(string $query): Generator } } + public function remove(Record $record): void + { + if (!isset($this->subjects[$this->recordKey($record)])) { + return; + } + + unset($this->subjects[$this->recordKey($record)]); + } + public function write(Record $record): void { $this->open(); - $this->subjects[$record->recordType().$record->identifier()] = [$record->recordType(), $record->identifier()]; + $this->subjects[$this->recordKey($record)] = [$record->recordType(), $record->identifier()]; if (++$this->counter % self::BATCH_SIZE === 0) { $this->flush(); @@ -107,4 +116,9 @@ private function open(): void $this->initialized = true; } + + private function recordKey(Record $record): string + { + return $record->recordType().$record->identifier(); + } } diff --git a/lib/Adapter/Php/InMemory/InMemorySearchIndex.php b/lib/Adapter/Php/InMemory/InMemorySearchIndex.php index 122fcb1..7ddc55a 100644 --- a/lib/Adapter/Php/InMemory/InMemorySearchIndex.php +++ b/lib/Adapter/Php/InMemory/InMemorySearchIndex.php @@ -36,4 +36,9 @@ public function write(Record $record): void public function flush(): void { } + + public function remove(Record $record): void + { + unset($this->buffer[$record->identifier()]); + } } diff --git a/lib/Model/SearchIndex/ValidatingSearchIndex.php b/lib/Model/SearchIndex/ValidatingSearchIndex.php new file mode 100644 index 0000000..e839722 --- /dev/null +++ b/lib/Model/SearchIndex/ValidatingSearchIndex.php @@ -0,0 +1,70 @@ +innerIndex = $innerIndex; + $this->index = $index; + } + + /** + * {@inheritDoc} + */ + public function search(string $query): Generator + { + foreach ($this->innerIndex->search($query) as $result) { + if (!$this->index->has($result)) { + $this->innerIndex->remove($result); + continue; + } + + $record = $this->index->get($result); + + if (!$record instanceof HasPath) { + yield $result; + return; + } + + if (!file_exists($record->filePath())) { + $this->innerIndex->remove($record); + continue; + } + + yield $result; + } + } + + public function write(Record $record): void + { + $this->innerIndex->write($record); + } + + public function remove(Record $record): void + { + $this->innerIndex->remove($record); + } + + public function flush(): void + { + $this->innerIndex->flush(); + } +} diff --git a/tests/Benchmark/SearchBench.php b/tests/Benchmark/SearchBench.php index 42e8362..c2d99d1 100644 --- a/tests/Benchmark/SearchBench.php +++ b/tests/Benchmark/SearchBench.php @@ -3,33 +3,56 @@ namespace Phpactor\Indexer\Tests\Benchmark; use Phpactor\Indexer\Adapter\Php\FileSearchIndex; +use Phpactor\Indexer\Adapter\Php\Serialized\FileRepository; +use Phpactor\Indexer\Adapter\Php\Serialized\SerializedIndex; use Phpactor\Indexer\Model\Matcher\ClassShortNameMatcher; +use Phpactor\Indexer\Model\SearchIndex; +use Phpactor\Indexer\Model\SearchIndex\ValidatingSearchIndex; /** + * Run ./bin/console index:build before running this benchmark + * * @Iterations(10) * @Revs(1) - * @OutputTimeUnit("milliseconds") + * @OutputTimeUnit("seconds") */ class SearchBench { /** - * @var FileSearchIndex + * @var SearchIndex */ private $search; + public function createFileSearch(): void + { + $indexPath = __DIR__ . '/../..'; + $this->search = new FileSearchIndex($indexPath . '/cache/search', new ClassShortNameMatcher()); + } + + public function createValidatingFileSearch(): void + { + $indexPath = __DIR__ . '/../../cache'; + $this->search = new ValidatingSearchIndex( + new FileSearchIndex($indexPath . '/search', new ClassShortNameMatcher()), + new SerializedIndex(new FileRepository($indexPath)) + ); + } + /** - * Run ./bin/console index:build before running this benchmark + * @BeforeMethods({"createFileSearch"}) + * @ParamProviders({"provideSearches"}) */ - public function setUp(): void + public function benchFileSearch(array $params): void { - $this->search = new FileSearchIndex(__DIR__ . '/../../cache/search', new ClassShortNameMatcher()); + foreach ($this->search->search($params['search']) as $result) { + } } /** - * @BeforeMethods({"setUp"}) + * @BeforeMethods({"createValidatingFileSearch"}) * @ParamProviders({"provideSearches"}) */ - public function benchSearch(array $params): void + public function benchValidatingFileSearch(array $params): void { foreach ($this->search->search($params['search']) as $result) { } @@ -45,10 +68,6 @@ public function provideSearches() 'search' => 'B', ]; - yield 'C' => [ - 'search' => 'C', - ]; - yield 'Request' => [ 'search' => 'Request', ]; From 43b8682bd17486cf9719e8224c68f33e21f33f46 Mon Sep 17 00:00:00 2001 From: Daniel Leech Date: Wed, 27 May 2020 21:25:29 +0100 Subject: [PATCH 3/6] Add phpbench to composer --- composer.json | 5 +++-- lib/Model/SearchIndex/FilteredSearchIndex.php | 5 +++++ tests/Benchmark/SearchBench.php | 4 ---- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/composer.json b/composer.json index 08e1438..53625ac 100644 --- a/composer.json +++ b/composer.json @@ -34,7 +34,7 @@ "phpactor/test-utils": "^1.0.2", "phpactor/console-extension": "^0.1.2", "symfony/var-dumper": "^5.1", - "phpbench/phpbench": "^1.0@dev" + "phpbench/phpbench": "^1.0" }, "autoload": { "psr-4": { @@ -56,7 +56,8 @@ "integrate": [ "vendor/bin/phpstan analyse", "vendor/bin/php-cs-fixer fix", - "vendor/bin/phpunit" + "vendor/bin/phpunit", + "vendor/bin/phpbench run" ] } } diff --git a/lib/Model/SearchIndex/FilteredSearchIndex.php b/lib/Model/SearchIndex/FilteredSearchIndex.php index cc39a89..54d0be9 100644 --- a/lib/Model/SearchIndex/FilteredSearchIndex.php +++ b/lib/Model/SearchIndex/FilteredSearchIndex.php @@ -49,4 +49,9 @@ public function flush(): void { $this->innerIndex->flush(); } + + public function remove(Record $record): void + { + $this->innerIndex->remove($record); + } } diff --git a/tests/Benchmark/SearchBench.php b/tests/Benchmark/SearchBench.php index c2d99d1..a3ed681 100644 --- a/tests/Benchmark/SearchBench.php +++ b/tests/Benchmark/SearchBench.php @@ -64,10 +64,6 @@ public function provideSearches() 'search' => 'A', ]; - yield 'B' => [ - 'search' => 'B', - ]; - yield 'Request' => [ 'search' => 'Request', ]; From b15fe5e825543d3565528ccc57341f775d872c28 Mon Sep 17 00:00:00 2001 From: Daniel Leech Date: Wed, 27 May 2020 21:26:20 +0100 Subject: [PATCH 4/6] Build index before integrating --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index e75c3c7..149c597 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,4 +14,5 @@ before_script: - composer install script: + - bin/console index:build - composer integrate From 3f0bddbd5ffee2f79edfd3b7e42c2d45da6e4874 Mon Sep 17 00:00:00 2001 From: Daniel Leech Date: Thu, 28 May 2020 17:55:20 +0100 Subject: [PATCH 5/6] Reimplemented in-memory index --- lib/Adapter/Php/InMemory/InMemoryIndex.php | 56 ++++++++---------- .../Php/InMemory/InMemoryRepository.php | 58 ------------------- 2 files changed, 23 insertions(+), 91 deletions(-) delete mode 100644 lib/Adapter/Php/InMemory/InMemoryRepository.php diff --git a/lib/Adapter/Php/InMemory/InMemoryIndex.php b/lib/Adapter/Php/InMemory/InMemoryIndex.php index ae21a2d..5a826e9 100644 --- a/lib/Adapter/Php/InMemory/InMemoryIndex.php +++ b/lib/Adapter/Php/InMemory/InMemoryIndex.php @@ -28,11 +28,19 @@ class InMemoryIndex implements Index */ private $searchIndex; - public function __construct(?InMemoryRepository $repository = null) + /** + * @var array + */ + private $index; + + /** + * @param array $index + */ + public function __construct(array $index = []) { - $this->repository = $repository ?: new InMemoryRepository(); $this->searchIndex = new InMemorySearchIndex(); $this->lastUpdate = 0; + $this->index = $index; } public function lastUpdate(): int @@ -47,43 +55,20 @@ public function query(): IndexQueryAgent public function write(Record $record): void { + $this->index[$this->recordKey($record)] = $record; $this->searchIndex->write($record); - - if ($record instanceof ClassRecord) { - $this->repository->putClass($record); - return; - } - - if ($record instanceof FunctionRecord) { - $this->repository->putFunction($record); - return; - } - - throw new RuntimeException(sprintf( - 'Do not know how to index "%s"', - get_class($record) - )); } - /** - * {@inheritDoc} - */ public function get(Record $record): Record { - if ($record instanceof ClassRecord) { - // @phpstan-ignore-next-line - return $this->repository->getClass($record->fqn()) ?? $record; - } + $key = $this->recordKey($record); - if ($record instanceof FunctionRecord) { - // @phpstan-ignore-next-line - return $this->repository->getFunction($record->fqn()) ?? $record; + if (isset($this->index[$key])) { + /** @phpstan-ignore-next-line */ + return $this->index[$key]; } - throw new RuntimeException(sprintf( - 'Do not know how to index "%s"', - get_class($record) - )); + return $record; } public function isFresh(SplFileInfo $fileInfo): bool @@ -93,7 +78,7 @@ public function isFresh(SplFileInfo $fileInfo): bool public function reset(): void { - $this->repository->reset(); + $this->index = []; } public function exists(): bool @@ -108,7 +93,7 @@ public function done(): void public function has(Record $record): bool { - return false; + return isset($this->index[$this->recordKey($record)]); } /** @@ -118,4 +103,9 @@ public function search(string $search): Generator { yield from $this->searchIndex->search($search); } + + private function recordKey(Record $record): string + { + return $record->recordType().$record->identifier(); + } } diff --git a/lib/Adapter/Php/InMemory/InMemoryRepository.php b/lib/Adapter/Php/InMemory/InMemoryRepository.php deleted file mode 100644 index a25a7ca..0000000 --- a/lib/Adapter/Php/InMemory/InMemoryRepository.php +++ /dev/null @@ -1,58 +0,0 @@ - - */ - private $classes = []; - - /** - * @var array - */ - private $functions = []; - - /** - * @var int - */ - public $lastUpdate = 0; - - public function putClass(ClassRecord $class): void - { - $this->classes[$class->fqn()->__toString()] = $class; - } - - public function putFunction(FunctionRecord $function): void - { - $this->functions[$function->fqn()->__toString()] = $function; - } - - public function getClass(string $fqn): ?ClassRecord - { - if (!isset($this->classes[$fqn])) { - return null; - } - - return $this->classes[$fqn]; - } - - public function reset(): void - { - $this->classes = []; - $this->functions = []; - } - - public function getFunction(string $fqn): ?FunctionRecord - { - if (!isset($this->functions[$fqn])) { - return null; - } - - return $this->functions[$fqn]; - } -} From 750a0c794428fdd0c9d3ee28e3de289055ba0c81 Mon Sep 17 00:00:00 2001 From: Daniel Leech Date: Thu, 28 May 2020 18:04:00 +0100 Subject: [PATCH 6/6] Adds test for validating search index --- lib/Adapter/Php/InMemory/InMemoryIndex.php | 14 +-- .../Php/InMemory/InMemorySearchIndex.php | 5 ++ .../SearchIndex/ValidatingSearchIndexTest.php | 87 +++++++++++++++++++ 3 files changed, 95 insertions(+), 11 deletions(-) create mode 100644 tests/Unit/Model/SearchIndex/ValidatingSearchIndexTest.php diff --git a/lib/Adapter/Php/InMemory/InMemoryIndex.php b/lib/Adapter/Php/InMemory/InMemoryIndex.php index 5a826e9..37e290f 100644 --- a/lib/Adapter/Php/InMemory/InMemoryIndex.php +++ b/lib/Adapter/Php/InMemory/InMemoryIndex.php @@ -5,21 +5,13 @@ use Generator; use Phpactor\Indexer\Model\Index; use Phpactor\Indexer\Model\IndexQueryAgent; -use RuntimeException; -use Phpactor\Indexer\Model\Record\FunctionRecord; -use Phpactor\Indexer\Model\Record\ClassRecord; use Phpactor\Indexer\Model\Record; use SplFileInfo; class InMemoryIndex implements Index { /** - * @var InMemoryRepository - */ - private $repository; - - /** - * @var int + * @var int|null */ private $lastUpdate; @@ -83,12 +75,12 @@ public function reset(): void public function exists(): bool { - return $this->repository->lastUpdate !== 0; + return $this->lastUpdate !== 0; } public function done(): void { - $this->repository->lastUpdate = time(); + $this->lastUpdate = time(); } public function has(Record $record): bool diff --git a/lib/Adapter/Php/InMemory/InMemorySearchIndex.php b/lib/Adapter/Php/InMemory/InMemorySearchIndex.php index 7ddc55a..ad17ed0 100644 --- a/lib/Adapter/Php/InMemory/InMemorySearchIndex.php +++ b/lib/Adapter/Php/InMemory/InMemorySearchIndex.php @@ -33,6 +33,11 @@ public function write(Record $record): void $this->buffer[$record->identifier()] = [$record->recordType(), $record->identifier()]; } + public function has(Record $record): bool + { + return isset($this->buffer[$record->identifier()]); + } + public function flush(): void { } diff --git a/tests/Unit/Model/SearchIndex/ValidatingSearchIndexTest.php b/tests/Unit/Model/SearchIndex/ValidatingSearchIndexTest.php new file mode 100644 index 0000000..5318e1a --- /dev/null +++ b/tests/Unit/Model/SearchIndex/ValidatingSearchIndexTest.php @@ -0,0 +1,87 @@ +innerSearchIndex = new InMemorySearchIndex(); + $this->index = new InMemoryIndex(); + $this->searchIndex = new ValidatingSearchIndex( + $this->innerSearchIndex, + $this->index + ); + } + + public function testWillRemoveResultIfNotExistIndex(): void + { + $record = ClassRecord::fromName('Foobar'); + $this->innerSearchIndex->write($record); + + self::assertSearchCount(0, $this->searchIndex->search('Foobar')); + self::assertFalse($this->innerSearchIndex->has($record)); + } + + public function testYieldsRecordsWithoutAPath(): void + { + $record = MemberRecord::fromIdentifier('method#foo'); + $this->index->write($record); + $this->innerSearchIndex->write($record); + + self::assertSearchCount(1, $this->searchIndex->search('method#foo')); + } + + public function testRemovesFromIndexIfFileDoesNotExist(): void + { + $record = ClassRecord::fromName('Foobar') + ->setFilePath($this->workspace()->path('nope.php')); + + $this->index->write($record); + $this->innerSearchIndex->write($record); + + self::assertSearchCount(0, $this->searchIndex->search('Foobar')); + self::assertFalse($this->innerSearchIndex->has($record)); + } + + public function testYieldsSearchResultIfFileExists(): void + { + $this->workspace()->put('yep.php', 'foo'); + $record = ClassRecord::fromName('Foobar') + ->setFilePath($this->workspace()->path('yep.php')); + + $this->index->write($record); + $this->innerSearchIndex->write($record); + + self::assertSearchCount(1, $this->searchIndex->search('Foobar')); + self::assertTrue($this->innerSearchIndex->has($record)); + } + + private static function assertSearchCount(int $int, Generator $generator): void + { + self::assertEquals($int, count(iterator_to_array($generator))); + } +}