Parallel indexing - #3065
Parallel indexing#3065AJenbo wants to merge 1 commit into
Conversation
| public function assign(array $paths): void | ||
| { | ||
| $this->chunk = $paths; | ||
| $this->input->write(Protocol::encode( |
There was a problem hiding this comment.
why use a custom protocol instead of f.e. json + newline? how much more efficient is this?
There was a problem hiding this comment.
It's not really for performance, unserializing() of objects is simpler and we don't have to worry about thing like UTF encoding and it matches the disk format so felt like less of a gamble.
| } | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
not necessarily for this PR but would be nice to show some information about the paraellelism in the command output before or after indexing (e.g. how many workers were utilised)
| if ($phpactorBin) { | ||
| $config[LanguageServerExtension::PARAM_PHPACTOR_BIN] = $phpactorBin; | ||
| // Workers do not shre the CWD with the main process so we must resolve relative paths. | ||
| $config[IndexerExtension::PARAM_WORKER_BIN] = realpath($phpactorBin) ?: $phpactorBin; |
There was a problem hiding this comment.
not sure exactly why this is necessary? Phactor is already spawning workers for other tasks and the CWD is passed as --working-dir?
phpactor/lib/Extension/LanguageServer/LanguageServerExtension.php
Lines 676 to 679 in d2f84a3
There was a problem hiding this comment.
We need it so that workers use the same version and runtime as the manager process. --working-dir doesn't help here since it sets where the worker resolves the project, but not where the executable is resolved.
dantleech
left a comment
dantleech
left a comment
There was a problem hiding this comment.
very nice. a couple of comments / questions
2 participants
I was a bit sad to see that Phpactor is the slowest to index a project, while not being the most CPU hungry, this addresses that by splitting indexing up between sub processes and merging the results in to a single index. Most of the dependencies for this was already there (amphp/process, symfony/process) so only fidry/cpu-core-counter was added as a new dependency.
This result in roughly 6x faster indexing cutting my test project from 3m30s to 35s. This does increase peak memory during indexing but not in an unexpected way.
The PR is a bit on the larger side so I could split it up in a refactor and then add the parallel indexer on top if preferred, but I would prefer not to...