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

Bug: Request & Response objects stored multiple times #4580

Closed
maddrid opened this issue Apr 17, 2021 · 5 comments
Closed

Bug: Request & Response objects stored multiple times #4580

maddrid opened this issue Apr 17, 2021 · 5 comments
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@maddrid
Copy link

maddrid commented Apr 17, 2021

Request & Response objects stored multiple times
CodeIgniter\Config\BaseService $instances
CodeIgniter\CodeIgniter $request,$response;
CodeIgniter\Controller $request,$response .

More objects share same duplication fate (Logger,Validation ... ... ) .

CodeIgniter 4 version
All

@maddrid maddrid added the bug Verified issues on the current code behavior or pull requests that will fix them label Apr 17, 2021
@MGatner
Copy link
Member

MGatner commented Apr 18, 2021

All the classes you referenced use Services to access a shared instance of Request and Response, which are passed around by reference between classes so it is always really just one instance. If you know something I don't feel free to reopen this issue, but what you've described so far is neither a bug nor accurate.

@MGatner MGatner closed this as completed Apr 18, 2021
@maddrid
Copy link
Author

maddrid commented Apr 18, 2021

passed around by reference between classes so it is always really just one instance

If we add that ci4 can be used inside another framework

 class myFramework {
...
 $this->app = require realpath($bootstrap) ?: $bootstrap;
}

means that Some objects are stored 4 times .
One large object Instance stored 3-4 times x (int) number of objects = Large memory usage .
As you can see below it holds ,configuration values , body... Uri object ... ...
Each time an object it's stored as a property in another class, occupies a portion of memory .
Better explained here https://ocramius.github.io/blog/zf2-and-symfony-service-proxies-with-doctrine-proxies/

request
works well without injecting the request and response .
Capture
Ps: is not about request and response objects only , it's about application design in general .

@MGatner
Copy link
Member

MGatner commented Apr 19, 2021

Each time an object it's stored as a property in another class, occupies a portion of memory

I don't believe this is accurate. I wasn't able to find information confirming or denying it (except for one StackOverflow from six years ago that was deemed a bug). But the article you linked doesn't apply to your statement at all.

Doctrine Proxies look cool but we won't be using them. The framework as a whole uses something akin to what that article seems "Service Location", but with some of the concerns addressed.

I still see no compelling evidence that our implementation represents any performance concerns.

@lonnieezell
Copy link
Member

Obviously we have no control over anyone else would choose to implement mixing CI with their own framework. However, if they choose to use the Services class then the shared instances is still a thing.

Unlike the example you pointed to, CI doesn't require passing an instance of a ServiceLocator class around, it uses static methods which keeps it at a single instance. If it did, there's still ways around getting multiple instances of a ServiceLocator class, one of which would be to create a single instance in your controller, and then manually inject the dependency. However, most frameworks have some form of DI container that handles instance control for you, so that shouldn't be a problem anyway. And when passed into a class/method, it's not the entire object that's being passed in, it's a class identifier that points to the object itself. See: https://www.php.net/manual/en/language.oop5.references.php

Unless you can provide a concrete example of this happening that is within the control of the framework I'll have to agree with @MGatner and say that I've seen no compelling evidence of this being an issue with the current implementation.

@maddrid
Copy link
Author

maddrid commented Apr 19, 2021

True . Investigating pivetta's contribution on this matter on zend framework , and doing some benchmarks on ci4 storing 4 -40 instances of( IncomingRequest and Response) or calling services each time , there is just a little difference in speed , and memory usage .
just drops from 2.77 to 2.70 Mb/ request .
Further investigation was not worth pursuing
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

No branches or pull requests

3 participants