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

Performance(medium, 1.13%): Lazily instantiate Enlight_Event_Handler_Default (would involve SW-PR) #67

Open
cjuner opened this issue Feb 8, 2018 · 3 comments
Labels
enhancement Issues that describe new features or improvements to existing features.

Comments

@cjuner
Copy link
Member

cjuner commented Feb 8, 2018

Shopware instantiates an instance of Enlight_Event_Handler_Default for every subscriber – even if the corresponding events are never triggered.

In our default demo shops with none of our plugins enabled, Shopware\Components\ContainerAwareEventManager->addListener is called 125 times (2.99% of request processing time), and with 25 of our plugins, 890 times (3.30% of request processing time). That makes for a lot of unnecessary instances. The event handler's constructor takes about 1 % and 1.13 % of total request processing time, respectively.

Instead, the handler could be lazily instantiated when necessary, i.e., when the event is first triggered. Until then, the constructor arguments could be cached.

https://github.com/VIISON/shopware/blob/939ed8f18cef093c20a79e4b45ca825923a30ec0/engine/Library/Enlight/Event/EventManager.php#L78

@cjuner cjuner added enhancement Issues that describe new features or improvements to existing features. refactoring Issues that describe one or more code changes that neither fix a bug nor change the business logic. labels Feb 8, 2018
@fixpunkt
Copy link
Member

fixpunkt commented Feb 8, 2018

1% of total response time doesn’t sound like it’s worth micro-optimizing 😉

@windaishi
Copy link

windaishi commented Feb 8, 2018

Take a look at the flight weight pattern :)

@cjuner cjuner changed the title Performance: Lazily instantiate Enlight_Event_Handler_Default Performance(1.13%): Lazily instantiate Enlight_Event_Handler_Default Feb 9, 2018
@cjuner cjuner changed the title Performance(1.13%): Lazily instantiate Enlight_Event_Handler_Default Performance(medium, 1.13%): Lazily instantiate Enlight_Event_Handler_Default (would involve SW-PR) Feb 9, 2018
@fixpunkt
Copy link
Member

fixpunkt commented Feb 9, 2018

I know the flyweight pattern, but that is still a specific optimization, which is probably not worth our time for 1%, but we'll see.

@fixpunkt fixpunkt removed the refactoring Issues that describe one or more code changes that neither fix a bug nor change the business logic. label Oct 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues that describe new features or improvements to existing features.
Projects
None yet
Development

No branches or pull requests

3 participants