Document lazy services and fix script doxygen

One IS template is sufficient :P

OK, what are they? Is there a drupal.org page explaining them?

This chat should help:

09:54 <Fabianx-screen> So the problem is:
09:54 <Fabianx-screen> Service A depends on Service B, which depends on C,D,E,F,G
09:55 <Fabianx-screen> However during normal operation Service A does not use Service B.
09:55 <p>  Why would A declare
09:55 <p> a dep on B if it doesn't use it?
09:55 <Fabianx-screen> because in some code paths it still uses it.
09:55 <Fabianx-screen> just not in the render code path.
09:55 <Fabianx-screen> "normal operation"
09:55 <Fabianx-screen> e.g. logging
09:56 <Fabianx-screen> you need it only for exception cases, but not injecting it would violate encapsulation policy.
09:56 <Fabianx-screen> So now suddenly the light service A depends on all the services of service B.
09:56 <Fabianx-screen> The trick?
09:56 <Fabianx-screen> Make Service B a proxy.
09:56 <Fabianx-screen> So lets say A) uses the log() method of service B.
09:57 <Fabianx-screen> Then the proxy implements:
09:57 <Fabianx-screen> function log() {
09:57 <Fabianx-screen>   $this->lazyLoadItself();
09:57 <Fabianx-screen>   $this->service->log(func_get_args());
09:57 <Fabianx-screen> }
09:57 <Fabianx-screen> very simplified.
09:57 <Fabianx-screen> And lazy load itself asks the container for the service.
09:58 <p> So rather than go through the overhead of constructing a service it won't use, it loads only when called
09:58 <Fabianx-screen> yes, exactly.
09:58 <Fabianx-screen> But does so 100% transparent to the service.
09:58 <p> And this was, presumably, a performance thing
09:58 <Fabianx-screen> So that not all services do not to implement.
09:59 <Fabianx-screen> public function log() { if (!isset($this>logger)) { $this->logger = $this->container->get('logger'); } $this->logger->log(...); }
09:59 <p> This makes a lot of sense
10:00 <Fabianx-screen> which would mean that all services would depend on the container, which would then end up being a super-object ...
10:00 <Fabianx-screen> yes, implemented for performance.
10:00 <Fabianx-screen> No other reason.
10:00 <Fabianx-screen> System can work 100% without proxies, just slower :)
10:00 <Fabianx-screen> I hope that helps :).
10:00 <p> And the lazy services are manually compiled, always?
10:01 <Fabianx-screen> Yes, however we created a script to do so.
10:01 <p> Gotcha.
10:01 <Fabianx-screen> It was done automatically at container build time, but that had several problems.
10:01 <p> What were those problems, out of curiosity?
10:01 <Fabianx-screen> for example modules that are not yet loaded when the container was loaded, yet we needed the interface of the modules ...
10:02 <Fabianx-screen> no autoloading, e.g. all proxies always loaded.
10:02 <Fabianx-screen> etc.

On a high level overview.

OK, so far, so good.

So what would a developer need to know about these? How do you create a lazy-loaded service proxy?

A developer will need to add "lazy: true" to their service definition in [module].services.yml and then would call e.g.:

./core/scripts/generate-proxy.sh 'Drupal\Core\Batch\BatchStorage' core/lib/Drupal/Core

      ->addUsage('\'Drupal\Core\Batch\BatchStorage\' "core/lib/Drupal/Core"')
      ->addUsage('\'Drupal\block\BlockRepository\' "core/modules/block/src"')
      ->addUsage('\'Drupal\mymodule\MyClass\' "modules/contrib/mymodule/src"');

etc.

Hm... Well this still isn't clear to me.

I do not see addUsage() in Core... and what is this generate-proxy.sh thing?

I was too lazy to type it all out:

./core/scripts/generate-proxy.sh 'Drupal\Core\Batch\BatchStorage' core/lib/Drupal/Core

will generate a proxy for the BatchStorage class.

./core/scripts/generate-proxy.sh 'Drupal\block\BlockRepository' core/modules/block/src

will generate a proxy for the BlockRepository class.

./core/scripts/generate-proxy.sh 'Drupal\mymodule\MyClass\' modules/contrib/mymodule/src

will generate a proxy for the custom MyClass class.

So it is always:

FullyQualifiedClassName Root-of-the-Namespace, e.g. for modules the 'src' directory.

This script does not exist. I see generate-proxy-class.php so I guess that is what you mean? That PHP file is awful -- it has no usage docs at all and some of the in-code comments are totally wrong like the line that says it's generating a database dump. Is there some plan to fix it up?

#9 Yes, that script.

Sorry :/

There are usage docs to what the Application provided via addUsage(), I am not sure how to add more.

Yes, the database script copy might have slipped through.

However on the plus side if you forget to run the script ( mark any service with lazy: true in core/config.services.yml or in any modules module.services.yml) and rebuild the drupal cache (drush cr) you will get very clear instructions how to run the script.

RE #10 - That generate-proxy.php file:

a) Has no @file block at the top

b) As far as I can see, has zero relevant/correct comments in it except the one line that says "// Bootstrap."

So. It really could use a @file block telling at least what it does and how to use it, and at least a fix to that comment that implies it is running a database dump command.

Let's make that part of this issue of "documenting lazy services"?

Anyway... Do you want to make a first pass at writing this documentation? I am still a bit at a loss. I was not able to run this script on my slightly outdated local test site to see what it would do, and I am pretty busy right now with other stuff so I do not have time to write it. I don't think it's a Novice issue... so if you want this to get done we will need to find someone to write up this documentation.

#11: Sorry, busy with Criticals, too.

Would it be possible when you triage the queue, that you ping again in 2 weeks or so?

And I agree the doxygen of the script should be fixed in here.

Maybe assign it to yourself so it doesn't fall underneath your issue queue? The unresolved Docs issue queue runs to about 9 pages of 50 issues right now. This will get lost -- in two weeks it will be buried with all the other issues that got reported and never acted upon. The only ones that get fixed are the ones that get marked Novice, and this one ... I don't think it can be. Sorry.

This sort of thing is why we need core integration of symfony/console. :-)

Thanks! That is a good start at the @file block for the script. A few thoughts:

1. +++ b/core/scripts/generate-proxy-class.php
   @@ -1,6 +1,16 @@
   + * For help, type this command:
   + * php core/scripts/generate-proxy-class.php -h generate-proxy-class
   

   Should probably say to do this from your Drupal root directory. And apparently also that you have to run it from an installed Drupal code base, because when I tried running this in my "commit stuff to Core" git repo (which is not connected to an installed site), I got an exception.

2. +++ b/core/scripts/generate-proxy-class.php
   @@ -1,6 +1,16 @@
   + * @see Drupal\Core\Command\GenerateProxyClassCommand
   

   The namespace in this line needs to start with \

3. Also... I went and looked at that class and there's basically no documentation there either. Should we add documenting that class to the scope of this issue?

   At a minimum, when the topic is written, both this file and that class should probably have @ingroup services in them.

Yah, it was a patch to get the conversation rolling. I'm not sure if this script is useful only for generating lazy services, or whether it has other uses.

#15.1: Added 'from your installed Drupal root...' Also, getting an exception instead of a help screen would be a bug if we had the console integration in place.

#15.2,3: I included the @see because it's reasonably readable code that shows you what's going on. It's probably not the best documentation effort. I'll just remove it. Also there's no @defgroup services but there is a @defgroup container, so I've added that. I'm not sure if it goes in @file, or if it should have its own docblock.

Still need a how-to writeup in core.api.php.

Looks good! Setting back to Needs Work since the topic has not yet been written.

tried with the latest patch as it not getting applied so, Rerolling the patch for the drupal version 9.4.x and attaching the Reroll_Diff along with that,

Patch applies to 10.1.x as well.

I'd RTBC but from #17: "Looks good! Setting back to Needs Work since the topic has not yet been written." I do not know what that means.

I filed a duplicate of this. On my issue I wrote that documentation should say:

- why a particular class needs to be proxied
- where the list of classes that get proxied is defined
- what triggers the proxy class generation -- is it something a human must do when these classes are changed, or is it automatic?

I'm reading the comments on this -- I'll try to write a docs topic based on them.

Random failure appears to be random ckeditor5 one. So LGTM

Not sure what topic either? Unless this needs a help_topic template but that wasn't around 7 years ago.

> Looks good! Setting back to Needs Work since the topic has not yet been written.

I assume this means the topic I've added to core.api.php with a @defgroup -- that's a documentation topic.

Committed/pushed to 10.1.x and cherry-picked back through 9.5.x, thanks!