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

Replace closures in config/service with factories #22

Open
lenada opened this issue Feb 9, 2015 · 4 comments
Open

Replace closures in config/service with factories #22

lenada opened this issue Feb 9, 2015 · 4 comments

Comments

@lenada
Copy link
Contributor

lenada commented Feb 9, 2015

when playing around with sysdig and php opcode cache,
I discovered that the 3 service config files are being read from the file system on every request.
The opcode cache does not hit because they all contain closures.
By using factories we will add a bunch of new files but it will perform better with opcode cache enabled.

@mtudor
Copy link
Owner

mtudor commented Feb 10, 2015

I think that's a good idea @lenada - and in fact service classes are easier for others to extend, also, so double bonus :-)

I'm happy to create the classes if you're happy to report back on correct caching once it's done?

@lenada
Copy link
Contributor Author

lenada commented Feb 10, 2015

@mtudor cool! I was wondering if you would mind moving away from the traits used by the Module and move towards a more classical zf2 Module approach? I will attach a PR ;)
Always happy to report back!

@mtudor
Copy link
Owner

mtudor commented Feb 10, 2015

I quite like the AbstractModule / Trait stuff actually - probably would rather stick with that as it makes organising config very straightforward and it's common across my modules. I don't think it's incompatible with also using service classes though? Assuming we're talking about the same thing!

Just need to take the closures out and extract them to service classes and then replace the reference to them in the services.config.php file.

I'm keen you not do any wasted work so you might like to hold off on doing too much until we've had a chance to discuss, or alternatively I can pop the factory classes together and you can do the checking for caching?

@lenada
Copy link
Contributor Author

lenada commented Feb 10, 2015

sounds good, you popping the factory classes together, I will test again 👍

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

No branches or pull requests

2 participants