theofidry / PsyshBundle

A command line REPL bundle for Symfony using PsySH.
MIT License
208 stars 29 forks source link

chore: Migrate away from ContainerAwareInterface + Allow Symfony 7 #80

Open aszenz opened 11 months ago

aszenz commented 11 months ago

Partially fixes https://github.com/theofidry/PsyshBundle/issues/79

SymfonyHackday

aszenz commented 8 months ago

@theofidry Could you check this pr

lstrojny commented 8 months ago

I’ve been running this patch in the real world for some time and I noticed a performance regression of up to 40ms on every request in environments where the bundle is enabled.

Because PsychBundle::boot() calls $this->container->get('psysh.facade'); the psych.shell service will be initialized on every request. The initialization of the facade used to be lazy but is now eager.

But: replacing it with a service closure works and makes it lazy again and removes the performance penalty.

diff --git a/resources/config/services.xml b/resources/config/services.xml
index 12617b5..4191ba1 100644
--- a/resources/config/services.xml
+++ b/resources/config/services.xml
@@ -21,8 +21,8 @@
         </service>

         <service id="psysh.facade" class="Fidry\PsyshBundle\PsyshFacade" public="true">
-            <call method="setShell">
-                <argument type="service" id="psysh.shell" />
+            <call method="setShellInitializer">
+                <argument type="service_closure" id="psysh.shell" />
             </call>
         </service>

diff --git a/src/PsyshFacade.php b/src/PsyshFacade.php
index c39f9a3..1a9c933 100644
--- a/src/PsyshFacade.php
+++ b/src/PsyshFacade.php
@@ -11,6 +11,7 @@

 namespace Fidry\PsyshBundle;

+use Closure;
 use Psy\Shell;
 use RuntimeException;
 use Symfony\Component\DependencyInjection\ContainerAwareInterface;
@@ -23,10 +24,15 @@ use function array_merge;
 final class PsyshFacade
 {
     private static Shell $shell;
+    private static Closure $shellInitializer;

     public static function init(): void
     {
-        // noop ... keeping the method as is for backward compatibility
+        if (isset(self::$shell)) {
+            return;
+        }
+
+        self::$shell = (self::$shellInitializer)();
     }

     public static function debug(array $variables = [], $bind = null): void
@@ -40,8 +46,8 @@ final class PsyshFacade
         self::$shell::debug($_variables, $bind);
     }

-    public function setShell(Shell $shell): void
+    public function setShellInitializer(Closure $shellInitializer): void
     {
-        self::$shell = $shell;
+        self::$shellInitializer = $shellInitializer;
     }
 }
theofidry commented 8 months ago

@lstrojny that would be a different scope however. But please check #81