From 6e50c5df5bce7115358eb14100afb9b0661bfca9 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Wed, 17 Aug 2022 17:37:00 +0200 Subject: [PATCH] Drop child process adapter While fully non-blocking the child process adapter is a factor 100 slower than all the other adapters combined, including the blocking fallback adapter. As such removing it wil increase performance for everyone not having ext-uv or ext-eio (coming in a follow-up PR) installed. Even at the cost of blocking for a short period of time. --- README.md | 18 ++----- composer.json | 3 +- composer.lock | 60 +++++++++++----------- src/ChildProcess/Adapter.php | 52 ------------------- src/ChildProcess/Directory.php | 75 ---------------------------- src/ChildProcess/File.php | 71 -------------------------- src/ChildProcess/NotExist.php | 75 ---------------------------- src/ChildProcess/StatTrait.php | 31 ------------ src/Factory.php | 4 -- tests/AbstractFilesystemTestCase.php | 5 -- 10 files changed, 34 insertions(+), 360 deletions(-) delete mode 100644 src/ChildProcess/Adapter.php delete mode 100644 src/ChildProcess/Directory.php delete mode 100644 src/ChildProcess/File.php delete mode 100644 src/ChildProcess/NotExist.php delete mode 100644 src/ChildProcess/StatTrait.php diff --git a/README.md b/README.md index 74b4703d..c9e28924 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,6 @@ * [Factory](#factory) * [create()](#create) * [Filesystem implementations](#filesystem-implementations) - * [ChildProcess](#childprocess) * [Uv](#uv) * [AdapterInterface](#adapterinterface) * [detect()](#detect) @@ -102,26 +101,15 @@ an implementation detail. You should use the [`Factory`](#factory) to automatically create a new instance. The factory will determine the most performant filesystem for your environment. Any extension based filesystem are -preferred before falling back to less performant filesystems. When no extensions are detected it will fall back to -the [`ChildProcess`](#childprocess) on Linux/Mac machines, and to an internal fallback filesystem for windows that -uses blocking system calls. This blocking filesystem isn't documented and will be removed once -the [`ChildProcess`](#childprocess) filesystem works on Windows. It's merely mentioned here for reference until then. +preferred before falling back to less performant filesystems. When no extensions are detected it will fall back to an +internal fallback filesystem that uses blocking system calls. As such it is highly recommended to install one of the +extensions that unlocks more performant filesystem operations. Advanced! If you explicitly need a certain filesystem implementation, you can manually instantiate one of the following classes. Note that you may have to install the required PHP extensions for the respective event loop implementation first or they will throw a `BadMethodCallException` on creation. -#### ChildProcess - -A [`child process`](https://reactphp.org/child-process/) based filesystem. - -This uses the blocking calls like the [`file_get_contents()`](https://www.php.net/manual/en/function.file-get-contents.php) -function to do filesystem calls and is the only implementation which works out of the box with PHP. - -Due to using child processes to handle filesystem calls, this filesystem the least performant is only used when no -extensions are found to create a more performant filesystem. - #### Uv An `ext-uv` based filesystem. diff --git a/composer.json b/composer.json index 52f0de90..27338eba 100644 --- a/composer.json +++ b/composer.json @@ -17,8 +17,7 @@ "react/event-loop": "^1.2", "react/promise": "^2.8", "react/promise-stream": "^1.2", - "react/stream": "^1.2", - "wyrihaximus/react-child-process-promise-closure": "^1.0" + "react/stream": "^1.2" }, "require-dev": { "clue/block-react": "^1.4", diff --git a/composer.lock b/composer.lock index b912387b..8c8aa373 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "f5ce03ce451f08ea50b1130047d65bfa", + "content-hash": "6e8fa47b318b0ad2d81ef9b4103e9033", "packages": [ { "name": "cakephp/core", @@ -413,12 +413,12 @@ } }, "autoload": { - "psr-4": { - "Opis\\Closure\\": "src/" - }, "files": [ "functions.php" - ] + ], + "psr-4": { + "Opis\\Closure\\": "src/" + } }, "notification-url": "https://packagist.org/downloads/", "license": [ @@ -890,12 +890,12 @@ }, "type": "library", "autoload": { - "psr-4": { - "React\\Promise\\Stream\\": "src/" - }, "files": [ "src/functions_include.php" - ] + ], + "psr-4": { + "React\\Promise\\Stream\\": "src/" + } }, "notification-url": "https://packagist.org/downloads/", "license": [ @@ -1565,12 +1565,12 @@ }, "type": "library", "autoload": { - "psr-4": { - "WyriHaximus\\": "src/" - }, "files": [ "src/functions_include.php" - ] + ], + "psr-4": { + "WyriHaximus\\": "src/" + } }, "notification-url": "https://packagist.org/downloads/", "license": [ @@ -1618,12 +1618,12 @@ }, "type": "library", "autoload": { - "psr-4": { - "WyriHaximus\\": "src/" - }, "files": [ "src/functions_include.php" - ] + ], + "psr-4": { + "WyriHaximus\\": "src/" + } }, "notification-url": "https://packagist.org/downloads/", "license": [ @@ -1805,12 +1805,12 @@ }, "type": "library", "autoload": { - "psr-4": { - "WyriHaximus\\React\\": "src/" - }, "files": [ "src/functions_include.php" - ] + ], + "psr-4": { + "WyriHaximus\\React\\": "src/" + } }, "notification-url": "https://packagist.org/downloads/", "license": [ @@ -1852,12 +1852,12 @@ }, "type": "library", "autoload": { - "psr-4": { - "WyriHaximus\\React\\": "src/" - }, "files": [ "src/functions_include.php" - ] + ], + "psr-4": { + "WyriHaximus\\React\\": "src/" + } }, "notification-url": "https://packagist.org/downloads/", "license": [ @@ -3821,12 +3821,12 @@ } }, "autoload": { - "psr-4": { - "Symfony\\Polyfill\\Ctype\\": "" - }, "files": [ "bootstrap.php" - ] + ], + "psr-4": { + "Symfony\\Polyfill\\Ctype\\": "" + } }, "notification-url": "https://packagist.org/downloads/", "license": [ @@ -3990,5 +3990,5 @@ "platform-overrides": { "php": "7.4.7" }, - "plugin-api-version": "2.2.0" + "plugin-api-version": "2.3.0" } diff --git a/src/ChildProcess/Adapter.php b/src/ChildProcess/Adapter.php deleted file mode 100644 index 3b0dcd8b..00000000 --- a/src/ChildProcess/Adapter.php +++ /dev/null @@ -1,52 +0,0 @@ -internalStat($path)->then(function (?Stat $stat) use ($path) { - if ($stat === null) { - return new NotExist($this, dirname($path) . DIRECTORY_SEPARATOR, basename($path)); - } - - switch (ModeTypeDetector::detect($stat->mode())) { - case Node\DirectoryInterface::class: - return $this->directory($stat->path()); - break; - case Node\FileInterface::class: - return $this->file($stat->path()); - break; - default: - return new Node\Unknown($stat->path(), $stat->path()); - break; - } - }); - } - - public function directory(string $path): Node\DirectoryInterface - { - return new Directory($this,dirname($path) . DIRECTORY_SEPARATOR, basename($path)); - } - - public function file(string $path): Node\FileInterface - { - return new File(dirname($path) . DIRECTORY_SEPARATOR, basename($path)); - } -} diff --git a/src/ChildProcess/Directory.php b/src/ChildProcess/Directory.php deleted file mode 100644 index df0f6100..00000000 --- a/src/ChildProcess/Directory.php +++ /dev/null @@ -1,75 +0,0 @@ -filesystem = $filesystem; - $this->path = $path; - $this->name = $name; - } - - public function stat(): PromiseInterface - { - return $this->internalStat($this->path . $this->name); - } - - public function ls(): PromiseInterface - { - $path = $this->path . $this->name; - return childProcessPromiseClosure(Loop::get(), function () use ($path): array { - return scandir($path); - })->then( function (array $contents): PromiseInterface { - $promises = []; - foreach ($contents as $node) { - if (in_array($node, ['.', '..'])) { - continue; - } - - $promises[] = $this->filesystem->detect($this->path . $this->name . DIRECTORY_SEPARATOR . $node); - } - - return all($promises); - }); - } - - public function unlink(): PromiseInterface - { - $path = $this->path . $this->name; - return childProcessPromiseClosure(Loop::get(), function () use ($path): array { - if (count(scandir($path)) > 0) { - return ['unlinked' => false]; - } - - return ['unlinked' => rmdir($path)]; - })->then(static fn (array $data) => (bool)$data['unlinked']); - } - - public function path(): string - { - return $this->path; - } - - public function name(): string - { - return $this->name; - } -} diff --git a/src/ChildProcess/File.php b/src/ChildProcess/File.php deleted file mode 100644 index dc3eabcd..00000000 --- a/src/ChildProcess/File.php +++ /dev/null @@ -1,71 +0,0 @@ -path = $path; - $this->name = $name; - } - - public function stat(): PromiseInterface - { - return $this->internalStat($this->path . $this->name); - } - - public function getContents(int $offset = 0 , ?int $maxlen = null): PromiseInterface - { - $path = $this->path . $this->name; - return childProcessPromiseClosure(Loop::get(), function () use ($path, $offset, $maxlen): array { - return ['contents' => file_get_contents($path, false, null, $offset, $maxlen ?? (int)stat($path)['size'])]; - })->then(static fn (array $data): string => $data['contents']); - } - - public function putContents(string $contents, int $flags = 0): PromiseInterface - { - // Making sure we only pass in one flag for security reasons - if (($flags & \FILE_APPEND) == \FILE_APPEND) { - $flags = \FILE_APPEND; - } else { - $flags = 0; - } - - $path = $this->path . $this->name; - return childProcessPromiseClosure(Loop::get(), function () use ($path, $contents, $flags): array { - return ['size_written' => file_put_contents($path, $contents, $flags)]; - })->then(static fn (array $data) => (int)$data['size_written']); - } - - public function unlink(): PromiseInterface - { - $path = $this->path . $this->name; - return childProcessPromiseClosure(Loop::get(), function () use ($path): array { - return ['unlinked' => unlink($path)]; - })->then(static fn (array $data) => (bool)$data['unlinked']); - } - - public function path(): string - { - return $this->path; - } - - public function name(): string - { - return $this->name; - } -} diff --git a/src/ChildProcess/NotExist.php b/src/ChildProcess/NotExist.php deleted file mode 100644 index 1040b063..00000000 --- a/src/ChildProcess/NotExist.php +++ /dev/null @@ -1,75 +0,0 @@ -filesystem = $filesystem; - $this->path = $path; - $this->name = $name; - } - - public function stat(): PromiseInterface - { - return $this->internalStat($this->path . $this->name); - } - - public function createDirectory(): PromiseInterface - { - $path = $this->path . $this->name; - return childProcessPromiseClosure(Loop::get(), function () use ($path): array { - return ['mkdir' => mkdir($path,0777, true)]; - })->then(fn (array $data): Node\DirectoryInterface => new Directory($this->filesystem, $this->path, $this->name)); - } - - public function createFile(): PromiseInterface - { - $file = new File($this->path, $this->name); - - return $this->filesystem->detect($this->path)->then(function (Node\NodeInterface $node): PromiseInterface { - if ($node instanceof Node\NotExistInterface) { - return $node->createDirectory(); - } - - return resolve($node); - })->then(function () use ($file): PromiseInterface { - return $file->putContents(''); - })->then(function () use ($file): Node\FileInterface { - return $file; - }); - } - - public function unlink(): PromiseInterface - { - // Essentially a No-OP since it doesn't exist anyway - return resolve(true); - } - - public function path(): string - { - return $this->path; - } - - public function name(): string - { - return $this->name; - } -} diff --git a/src/ChildProcess/StatTrait.php b/src/ChildProcess/StatTrait.php deleted file mode 100644 index 287a197c..00000000 --- a/src/ChildProcess/StatTrait.php +++ /dev/null @@ -1,31 +0,0 @@ -then(function (array $stat) use ($path): ?Stat { - if (count($stat) > 0) { - return new Stat($path, $stat); - } - - return null; - }); - } -} diff --git a/src/Factory.php b/src/Factory.php index b204ec8f..94880c5d 100644 --- a/src/Factory.php +++ b/src/Factory.php @@ -17,10 +17,6 @@ public static function create(): AdapterInterface return new Uv\Adapter(); } - if (DIRECTORY_SEPARATOR !== '\\') { - return new ChildProcess\Adapter(); - } - return new Fallback\Adapter(); } } diff --git a/tests/AbstractFilesystemTestCase.php b/tests/AbstractFilesystemTestCase.php index 0e46e14b..7d6cd6b5 100644 --- a/tests/AbstractFilesystemTestCase.php +++ b/tests/AbstractFilesystemTestCase.php @@ -3,7 +3,6 @@ namespace React\Tests\Filesystem; use React\EventLoop; -use React\Filesystem\ChildProcess; use React\Filesystem\Fallback; use React\Filesystem\Uv; use PHPUnit\Framework\TestCase; @@ -25,10 +24,6 @@ final public function provideFilesystems(): iterable yield 'fallback' => [new Fallback\Adapter()]; - if (DIRECTORY_SEPARATOR !== '\\') { - yield 'childprocess' => [new ChildProcess\Adapter()]; - } - if (\function_exists('uv_loop_new') && $loop instanceof ExtUvLoop) { yield 'uv' => [new Uv\Adapter()]; }