@jrobeson pointed me here.

The functionality definitely sounds valuable. I know Lukas has been talking about this for a long time. However, the dependency chain here is not fun.

I cannot speak for Fabien, but I'd be wary of adding a dependency on 2 additional libraries to DependencyInjection just for this functionality. Even that aside, the cg-library package is licensed under Apache 2. That's a problem. Specifically, it means that the DI Component cannot run without Apache 2 code, which in turn means that it has the same impact license-wise as being Apache 2. That is, it becomes incompatible with GPLv2 and GPLv2+ code.

While Symfony itself is not GPL licensed, many of the systems that are now depending on it are. In particular, Drupal and phpBB are both GPLv2+ licensed. GPLv2 is incompatible with Apache 2. That would make the DI component, and by extension Symfony fullstack, unusable for both Drupal and phpBB, as well as likely others. (I don't know the license of any of the other Symfony-based systems off hand.) It also makes it unsuable for bespoke systems by shops that use GPLv2 (and there are a lot of them) that want to build off of Symfony2.

So, yeah. Introducing an Apache2 dependency would be a massive license-breaking and ecosystem-crippling change. :-/ jrobeson indicated that there was an alternative library, or that the cg-library could be relicensed if we convince the maintainer to do so. Either way the licensing issue would go away. The question of introducing additional dependencies I leave to fabpot.

Disclaimer: I'm the former Director of Legal Affairs for the Drupal Association, so I've spent a fair bit of time looking into this subject. I'm not just making it up. :-)

@Crell I've been told that today, and didn't reallly expect it (I have no idea whatsoever this involves, I write code :( ). If BSD-3-Clause is compliant, I can convert my library to useZend\Code instead. That would take me a couple of days, but is no problem at all.

@Crell I removed usage of cg-lib in ProxyManager as ofOcramius/ProxyManager#18 - should be ok now

Do you have any benchmark on a "classic" Symfony2 application ? (withoutsleep ;) ).

@lyrixx no, I actually don't use Symfony SE. You can actually benchmark it yourself by insulating a portion of your object graph that is not used and marking the root node of that portion as lazy. From what I know, the security component should be quite large.

Another thing this tries to remove is excessive usage of service location for performance optimizations, as I've explainedhere.

You can also read a detailed description of how lazy loading proxies (as implemented here) workhere

OOTB, as it is now, this PR does not introduce any overhead.

Your current implementation breaks on 1 point: recreating the proxy class when the original class changes. The class has to be added as a container resource for lazy services so that the container is dumped again when the class changes, so that the proxies are dumped again. Otherwise, the proxy might not match the class anymore.

What you would need is the logic ofContainerBuilder::addObjectResource but with a class name rather than an instance (which is easy asaddObjectResource could then be rewritten as$this->addClassResource(get_class($object))

For the ContainerBuilder case, usingeval may be better than creating a temporary file on the fly and requiring it. You don't gain anything about caching the generated code (as you regenerate it each time) except potential race conditions (see Doctrine considering to change the generation of proxies when using auto_generate_proxies).
Btw, Twig is already usingeval when you disable the template cache.

Your code has a big issue: it creates a new proxy instance each time the service is retrieved from the container instead of reusing the same instance for container-scoped services. So$container->get('router') !== $container->get('router') when marking it as lazy.

thus, getting the lazy service, then doing a method call (thus initializing the proxy) and then getting it again would give you the wrapped instance after that, being inconsistent.

@stof very good catch! Couldn't write a for that test because of the conflicting container names. I'll see if I can change the name of a generated container name and write something that covers that.

Also note, that the usage of eval may make the symfony2 projects that use lazy loading unusable on some shared hosts.

@mvrhov ouch! Do they turn it off? I can build a switch for that eventually if there's a way of recognizing it.

yeah. I worked for one a few years ago and we were disabling it. The problem was that that a large amount of code injections that were happening then abused eval to send a spam or worse. Trying to get unlisted from the sites that list spam sending IPs every few weeks also become troublesome.
There is disable_functions ini setting where all disabled functions a re listed.

@mvrhov I spawnedOcramius/ProxyManager#24 from that. It should not be handled in symfony itself.

@stof can you clarify when I should add a resource? I addedaddClassResource to the API, but now I'm wondering about what should trigger it. Should I do it inContainerBuilder#register? Are there other locations where definitions are injected?

The feature by itself would be a great addition but I agree with@Crell that the added dependencies are not so good.
Being able to reuse the CG library (if license compliant of course) would be a little bit better as it is already used by some Symfony2 bundles.

Besides, the generated class names of proxies should follow the naming convention used by both the CG library and Doctrine (with a__CG__ prefix and all, seeClassUtils) as it allows easily identifying proxies, wherever they come from, without having to test for all possible proxies interfaces out there.

To clarify, I modified the PR so that the dependencies are nowoptional.

@jalliot for CG Lib, I kicked that out and replaced it withZend\Code, which is BSD-3-clause and therefore compatible. That's quite final, since I don't want to get back and rewritethis backwards without any real benefit.

That's anyway stuff that doesn't affect this PR's code directly. SF2 just benefits from the abstraction layer of ProxyManager and should not really care about how that is achieved except for BC breaks, performance and security issues (and license compatibility, as introduced by@Crell).

For__CG__ I explicitly wanted to use__PM__ (or something different) because ProxyManager allows to hook in its own autoloader with its own rules depending on the naming strategy set in the proxy manager configuration. If that autoloader conflicts with the one in doctrine common, we have a problem. We don't want that, do we? :)

If you want to identify a ProxyManager proxy, you can use theutility within ProxyManager itself. An idea would be to drop the__PM__ constant from there so that you're forced to use the inflector instance and don't rely on a convention anymore.

its an optional dependency but i agree that adding 2 more dependencies can be a concern. doctrine proxy and the cg lib are commonly used already in the Symfony2 ecosystem while proxy manager and zend code are less used.

i didnt check but ideally the code should maybe gracefully ignore the lazy setting in case the dependencies are missing so that no code would break if those optional deps are not installed.

@lsmith77 perfect, I will add the code required to let any lazy markers being ignored in the case where dependencies are not loaded.

Still need to fix the 5.3 failures

To clarify my comments above, the proxy manager is an optional dependency and should therefore be hidden in an adapter, maybe even in its own Bridge, implementing a generic interface from DependencyInjection component

Agreed - fixing later today

I've opened#7890 with the suggestions of@beberlei

closing in favor of#7890