Merge pull request #15 from reactphp-parallel/dont-wait-for-void-call-results

WyriHaximus · web-flow · commit 69e76cabbab7 · 2020-10-04T19:17:44.000+02:00
Don't wait for void calls results
diff --git a/Makefile b/Makefile
@@ -40,7 +40,7 @@ psalm: ## Run static analysis (Psalm)
 	$(DOCKER_RUN) vendor/bin/psalm --threads=$(shell nproc) --shepherd --stats
 
 unit: ## Run tests
-	$(DOCKER_RUN) vendor/bin/phpunit --colors=always -c phpunit.xml.dist --coverage-text --coverage-html covHtml --coverage-clover ./build/logs/clover.xml
+	$(DOCKER_RUN) vendor/bin/phpunit --colors=always -c phpunit.xml.dist --coverage-text --coverage-html covHtml --coverage-clover ./build/logs/clover.xml --filter metricsDestructionTesting
 
 unit-ci: unit
 	if [ -f ./build/logs/clover.xml ]; then wget https://scrutinizer-ci.com/ocular.phar && sleep 3 && php ocular.phar code-coverage:upload --format=php-clover ./build/logs/clover.xml; fi
diff --git a/src/AbstractGeneratedProxy.php b/src/AbstractGeneratedProxy.php
@@ -7,6 +7,7 @@
 use parallel\Channel;
 use ReactParallel\ObjectProxy\Message\Call;
 use ReactParallel\ObjectProxy\Message\Destruct;
+use ReactParallel\ObjectProxy\Message\Notify;
 
 use function spl_object_hash;
 
@@ -24,7 +25,7 @@ final public function __construct(Channel $out, string $hash)
     /**
      * @param mixed[] $args
      *
-     * @return mixed
+     * @return mixed|void
      */
     final protected function proxyCallToMainThread(string $method, array $args)
     {
@@ -43,6 +44,20 @@ final protected function proxyCallToMainThread(string $method, array $args)
         return $result;
     }
 
+    /**
+     * @param mixed[] $args
+     */
+    final protected function proxyNotifyMainThread(string $interface, string $method, array $args): void
+    {
+        $this->out->send(new Notify(
+            $this->hash,
+            spl_object_hash($this),
+            $interface,
+            $method,
+            $args,
+        ));
+    }
+
     final protected function notifyMainThreadAboutDestruction(): void
     {
         try {
diff --git a/src/Composer/Installer.php b/src/Composer/Installer.php
@@ -13,8 +13,10 @@
 use Composer\Plugin\PluginInterface;
 use Composer\Script\Event;
 use Composer\Script\ScriptEvents;
+use PhpParser\Lexer\Emulative;
 use PhpParser\ParserFactory;
 use PhpParser\PrettyPrinter\Standard;
+use Psr\Log\LoggerInterface;
 use ReflectionClass;
 use Rx\Observable;
 use Throwable;
@@ -75,6 +77,7 @@ public static function generateProxies(Event $event): void
         $start    = microtime(true);
         $io       = $event->getIO();
         $composer = $event->getComposer();
+        $rootPath = self::locateRootPackageInstallPath($composer->getConfig(), $composer->getPackage());
 
         if (! function_exists('React\Promise\Resolve')) {
             /** @psalm-suppress UnresolvableInclude */
@@ -124,7 +127,7 @@ public static function generateProxies(Event $event): void
         $io->write('<info>react-parallel/object-proxy:</info> Locating interfaces');
 
         $installPath = self::locateRootPackageInstallPath($composer->getConfig(), $composer->getPackage()) . '/src/Generated/';
-        $proxies     = self::getProxies($composer, $io);
+        $proxies     = self::getProxies($composer, $io, $rootPath);
 
         $io->write('<info>react-parallel/object-proxy:</info> Found ' . count($proxies) . ' interface(s) and generated a proxy for each of them');
 
@@ -141,7 +144,7 @@ public static function generateProxies(Event $event): void
                 "['%s']",
                 '%s',
                 file_get_contents(
-                    self::locateRootPackageInstallPath($composer->getConfig(), $composer->getPackage()) . '/etc/ProxyList.php'
+                    $rootPath . '/etc/ProxyList.php'
                 )
             ),
             var_export($interfaces, TRUE_)
@@ -174,9 +177,9 @@ private static function locateRootPackageInstallPath(
     /**
      * @return array<InterfaceProxier>
      */
-    private static function getProxies(Composer $composer, IOInterface $io): array
+    private static function getProxies(Composer $composer, IOInterface $io, string $rootPath): array
     {
-        $phpParser = $parser = (new ParserFactory())->create(ParserFactory::PREFER_PHP7);
+        $phpParser = $parser = (new ParserFactory())->create(ParserFactory::PREFER_PHP7, new Emulative(['comments' => true]));
 
         $result     = [];
         $packages   = $composer->getRepositoryManager()->getLocalRepository()->getCanonicalPackages();
@@ -189,14 +192,22 @@ private static function getProxies(Composer $composer, IOInterface $io): array
             static fn (array $interfaces): Observable => observableFromArray(array_unique(array_values($interfaces)))
         )->map(
             /** @phpstan-ignore-next-line */
-            static function (string $interface) use ($io, $phpParser): ?array {
+            static function (string $interface) use ($io, $phpParser, $rootPath): ?array {
                 $io->write(sprintf('<info>react-parallel/object-proxy:</info> Creating proxy for %s', $interface));
 
                 /**
                  * @psalm-suppress ArgumentTypeCoercion
                  * @phpstan-ignore-next-line
                  */
-                return $phpParser->parse(file_get_contents((new ReflectionClass($interface))->getFileName()));
+                $fileName = (new ReflectionClass($interface))->getFileName();
+                if ($interface === LoggerInterface::class) {
+                    $fileName = $rootPath . '/vendor/psr/log/Psr/Log/LoggerInterface.php';
+                }
+
+                /**
+                 * @phpstan-ignore-next-line
+                 */
+                return $phpParser->parse(file_get_contents($fileName));
             }
         )->filter(
             static fn (?array $ast): bool => is_array($ast)
diff --git a/src/Composer/InterfaceProxier.php b/src/Composer/InterfaceProxier.php
@@ -5,7 +5,7 @@
 namespace ReactParallel\ObjectProxy\Composer;
 
 use PhpParser\Builder\Method;
-use PhpParser\Comment\Doc;
+use PhpParser\Comment;
 use PhpParser\Node;
 use ReactParallel\ObjectProxy\AbstractGeneratedProxy;
 
@@ -15,6 +15,9 @@
 use function is_array;
 use function property_exists;
 use function str_replace;
+use function strpos;
+
+use const WyriHaximus\Constants\Boolean\FALSE_;
 
 final class InterfaceProxier
 {
@@ -180,7 +183,7 @@ private function populateMethod(Node\Stmt\ClassMethod $method): Node\Stmt\ClassM
 
         $methodBody = new Node\Expr\MethodCall(
             new Node\Expr\Variable('this'),
-            'proxyCallToMainThread',
+            $this->isMethodVoid($method) ? 'proxyNotifyMainThread' : 'proxyCallToMainThread',
             [
                 new Node\Arg(
                     new Node\Expr\ConstFetch(
@@ -199,21 +202,32 @@ private function populateMethod(Node\Stmt\ClassMethod $method): Node\Stmt\ClassM
             $this->wrapMethodBody($method, $methodBody),
         ];
 
-        $method->setDocComment(new Doc(''));
-
         return $method;
     }
 
     private function wrapMethodBody(Node\Stmt\ClassMethod $method, Node\Expr\MethodCall $methodBody): Node\Stmt
+    {
+        if ($this->isMethodVoid($method)) {
+            return new Node\Stmt\Expression($methodBody);
+        }
+
+        return new Node\Stmt\Return_($methodBody);
+    }
+
+    private function isMethodVoid(Node\Stmt\ClassMethod $method): bool
     {
         /**
          * @psalm-suppress PossiblyInvalidCast
          * @phpstan-ignore-next-line
          */
-        if ((string) $method->getReturnType() !== 'void') {
-            return new Node\Stmt\Return_($methodBody);
-        }
+        return (string) $method->getReturnType() === 'void' ? true : $this->isMethodVoidFromDocBlock($method);
+    }
 
-        return new Node\Stmt\Expression($methodBody);
+    private function isMethodVoidFromDocBlock(Node\Stmt\ClassMethod $method): bool
+    {
+        /**
+         * @psalm-suppress PossiblyNullReference
+         */
+        return $method->getDocComment() instanceof Comment && strpos($method->getDocComment()->getText(), '@return void') !== FALSE_;
     }
 }
diff --git a/src/Message/Notify.php b/src/Message/Notify.php
@@ -0,0 +1,57 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ReactParallel\ObjectProxy\Message;
+
+final class Notify
+{
+    private string $hash;
+    private string $objectHash;
+    private string $interface;
+
+    private string $method;
+
+    /** @var mixed[] */
+    private array $args;
+
+    /**
+     * @param mixed[] $args
+     */
+    public function __construct(string $hash, string $objectHash, string $interface, string $method, array $args)
+    {
+        $this->hash       = $hash;
+        $this->objectHash = $objectHash;
+        $this->interface  = $interface;
+        $this->method     = $method;
+        $this->args       = $args;
+    }
+
+    public function hash(): string
+    {
+        return $this->hash;
+    }
+
+    public function objectHash(): string
+    {
+        return $this->objectHash;
+    }
+
+    public function interface(): string
+    {
+        return $this->interface;
+    }
+
+    public function method(): string
+    {
+        return $this->method;
+    }
+
+    /**
+     * @return mixed[]
+     */
+    public function args(): array
+    {
+        return $this->args;
+    }
+}
diff --git a/src/Proxy.php b/src/Proxy.php
@@ -9,6 +9,7 @@
 use ReactParallel\ObjectProxy\Generated\ProxyList;
 use ReactParallel\ObjectProxy\Message\Call;
 use ReactParallel\ObjectProxy\Message\Destruct;
+use ReactParallel\ObjectProxy\Message\Notify;
 use ReactParallel\ObjectProxy\Proxy\Instance;
 use WyriHaximus\Metrics\Label;
 use WyriHaximus\Metrics\Registry;
@@ -26,6 +27,7 @@ final class Proxy extends ProxyList
 
     private Factory $factory;
     private ?Counters $counterCreate   = null;
+    private ?Counters $counterNotify   = null;
     private ?Counters $counterCall     = null;
     private ?Counters $counterDestruct = null;
 
@@ -59,6 +61,12 @@ public function withMetrics(Registry $registry): self
             new Label\Name('class'),
             new Label\Name('interface'),
         );
+        $self->counterNotify   = $registry->counter(
+            'react_parallel_object_proxy_notify',
+            'The number of notifications from worker threads through proxies to the main thread',
+            new Label\Name('class'),
+            new Label\Name('interface'),
+        );
         $self->counterDestruct = $registry->counter(
             'react_parallel_object_proxy_destruct',
             'Number of destroyed proxies by the garbage collector',
@@ -103,6 +111,10 @@ public function __destruct()
     private function setUpHandlers(): void
     {
         $this->factory->streams()->channel($this->in)->subscribe(function (object $message): void {
+            if ($message instanceof Notify) {
+                $this->handleNotify($message);
+            }
+
             if ($message instanceof Call) {
                 $this->handleCall($message);
             }
@@ -117,6 +129,22 @@ private function setUpHandlers(): void
         });
     }
 
+    private function handleNotify(Notify $notify): void
+    {
+        if (! array_key_exists($notify->hash(), $this->instances)) {
+            return;
+        }
+
+        $instance = $this->instances[$notify->hash()];
+        $instance->reference($notify->objectHash());
+        if ($this->counterNotify instanceof Counters) {
+            $this->counterNotify->counter(new Label('class', $instance->class()), new Label('interface', $instance->interface()))->incr();
+        }
+
+        /** @phpstan-ignore-next-line */
+        $instance->object()->{$notify->method()}(...$notify->args());
+    }
+
     private function handleCall(Call $call): void
     {
         if (! array_key_exists($call->hash(), $this->instances)) {
diff --git a/tests/ProxyTest.php b/tests/ProxyTest.php
@@ -50,7 +50,7 @@ public function logger(): void
 
                 return $time;
             }, [$loggerProxy, $time])->always(static function () use ($factory): void {
-                $factory->lowLevelPool()->kill();
+                $factory->lowLevelPool()->close();
             }),
             $loop
         );
@@ -87,7 +87,7 @@ public function loggerThroughContainer(): void
 
                 return $time;
             }, [$containerProxy, $time])->always(static function () use ($factory): void {
-                $factory->lowLevelPool()->kill();
+                $factory->lowLevelPool()->close();
             }),
             $loop
         );
@@ -133,8 +133,7 @@ public function metricsDestructionTesting(): void
                 all($promises)->always(static function () use ($limitedPool): void {
                     $limitedPool->close();
                 }),
-                $loop,
-                30
+                $loop
             );
             self::assertCount(count($promises), $results);
         } catch (TimeoutException $timeoutException) {
