Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lazy-load on a per-property base (removes doctrine proxies, replaced by ProxyManager) #1241

Closed

Conversation

Ocramius
Copy link
Member

@Ocramius Ocramius commented Jan 9, 2015

This is just a proof of concept that I hacked together real quick.

This changes how proxies are generated and lazy-loaded in a very radical way:

  • method calls do not cause lazy-loading
  • lazy-loading is triggered by access to non-transient properties (things that need to be lazy-loaded)
  • access to identifiers or non-mapped properties won't cause lazy-loading
  • you can now use $class = get_class($proxy); $object = new $class(); without any side-effects, as the constructor logic is preserved

Following BC breaks need to be fixed or discussed before going forward on this idea:

  • proxies don't implement Doctrine\Common\Proxy\Proxy anymore, but ProxyManager\Proxy\GhostObjectInterface (BC break, needs fixing, can be easily done with some effort) BC break is voluntary
  • serialize($proxy) now causes proxy initialization (probably needs fixing, as this is a major BC break) - BC break is voluntary

Pending TODOs:

To give you an example, it is impossible (without reflection) to cause lazy-loading for the following class:

class Counter
{
    private $id;
    private $otherUnrelatedThing;
    public function doThings() { /* empty body (on purpose) */ }
}

Review on Reviewable

@doctrinebot
Copy link

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DDC-3481

We use Jira to track the state of pull requests and the versions they got
included in.

@goetas
Copy link
Member

goetas commented Jan 9, 2015

hi! If you are considering to release a 3.0 version:

@Ocramius
Copy link
Member Author

Ocramius commented Jan 9, 2015

I'm not yet considering 3.0. This is just a PR that I opened that is obviously incompatible with 2.x for now.

3.0 will happen, but only after 2.5 and further discussion about what has to land in it.

@stof
Copy link
Member

stof commented Jan 17, 2015

serialize($proxy) now causes proxy initialization (probably needs fixing, as this is a major BC break)

IMO, this is fine. If your entity already implements Serializable, it is already initialized when serializing (because of a method call). And if it does not and the proxy is not initialized yet, unserializing it gives you a broken object (because you don't have the initializer closure to initialize the proxy)

@stof
Copy link
Member

stof commented Jan 17, 2015

However, the Proxy interface BC break is an issue. Couldn't we still implement this interface ?

@Ocramius
Copy link
Member Author

@stof my idea is to modify the code generators to align to the existing interface: not yet sure how.

As for serialize(), the issue is a bit more serious right now, as serializing an object referencing (for example) a complex object graph pretty much gives you the entire DB in serialized string format :-)

I'm fine with breaking BC here though: we had people serializing proxies since ages, and we always told them not to do it; time to stop that.

@beberlei
Copy link
Member

Not implementing Proxy interface is a bit harsh, many third party code check for this, for example Symfony.

@Ocramius
Copy link
Member Author

@beberlei I plan to implement Proxy, just not sure how (yet)

@Ocramius
Copy link
Member Author

Just hooking in @alcaeus' suggestion for partial hydration of proxies: https://gist.github.com/alcaeus/fcc12672d7e46faa23e6c35516802e7b#file-proxy-php-L22

@Ocramius
Copy link
Member Author

This patch needs to be re-applied on top of develop

@Ocramius Ocramius changed the title [3.0] [POC] lazy-load on a per-property base Lazy-load on a per-property base (removes doctrine proxies, replaced by ProxyManager) Jun 24, 2017
@nsaleh
Copy link

nsaleh commented Aug 8, 2017

Hey,

is there any update on this? Would be a great improvement in some contexts.

regards,
Nabil

@Ocramius
Copy link
Member Author

Ocramius commented Aug 8, 2017

@nsaleh I didn't work further on this, but I will do that as soon as I can. If you want to give it a shot yourself, be my guest!

@nsaleh
Copy link

nsaleh commented Aug 8, 2017

Thanks for your quick answer and your work!, i might give it a try, but yeah time is a problem here too. If it would help and be appropriate to do maybe I could get some sponsoring, let me know.

As I understand this feature will be shipped with 3.0 is "done when it's done" right?

@Ocramius
Copy link
Member Author

Ocramius commented Aug 8, 2017 via email

@Ocramius
Copy link
Member Author

This was moved to #6719

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants