-
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Speed up - too much (unnecessary) includes #269
Comments
@franzliedke any ideas how we would defer inclusion of some of those files by composer, or whether it's even worth doing? |
Composer already only loads the files it needs. A few causes for this high amount:
As to lowering the amount. Laravel natively supports deferred service providers, causing the provider to be only loaded when any class in the |
Solution for swiftmailer?
|
@gooof Thanks for raising this issue. I will have a look at this for the next beta release after the upcoming beta.8. Out of interest: a) how did you notice this, and b) what did you use to get the list of included files? Re. service providers: At Laracon, I talked to Taylor about deferred service providers. He agreed they are mostly useless nowadays, and might be removed in a future release. (Memory savings - probably not, CPU cycle savings - meh.) They won't help much with this problem anyway: with well-written service providers, you will only avoid loading the service providers itself. Thanks to the IoC container, services that aren't needed for a request should not be instantiated, and thus not loaded. What we see here is probably due to two things: Not very hard to fix, but we'll have to go through all service providers and extensions. |
@franzliedke I notice that the API need 0.2sec to load my 3 groups on localhost and that every page has the same response time. While finding out from what the slowness comes, I'll use get_included_files() before the output happens. A hack is using a own class that call a class only if its really called. Extensions have a similar problem, in the bootstrap.php should be the event name defined where the class is required. |
To add to the Most Laravel providers (including the mailer) already use singletons that should ideally never resolve if the feature isn't actually used. Right now it's not the case as email drivers are loaded for every request because of the above... |
The event subscriber approach means that dependencies have to be injected (and thus instantiated, along with all *their* dependencies) at the time of registering event listeners - even when events are never fired within a request's lifecycle. This is unnecessary and causes more classes than necessary to be loaded. In this case, we can explicitly register event listeners that will resolve their dependencies when the event is fired, not before. Refs #1578.
Refs flarum/core#1578. Fixes flarum/framework#1703.
@gooof Can I ask you to share your setup that got you the list of included files in your original post? I am asking so that I can get something reproducible to test my progress. 😃 |
@gooof If you could provide the script that you used to test load time and included files, that would be great 🙂. I tried to get the included files myself as well. This is the output of Included Files / No Extensions (497)
Included Files / No Extensions / Debug (621)
|
The event subscriber approach means that dependencies have to be injected (and thus instantiated, along with all *their* dependencies) at the time of registering event listeners - even when events are never fired within a request's lifecycle. This is unnecessary and causes more classes than necessary to be loaded. In this case, we can explicitly register event listeners that will resolve their dependencies when the event is fired, not before. Refs #1578.
This comment has been minimized.
This comment has been minimized.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum. |
Bug Report
Current Behavior
Currently flarum is loading over 450 files per page (with standard extensions enabled).
That is a bit too much and costs much unnecessary time.
Example: swiftmailer are never need on the frontend, its only to send mails in cronjobs, after sending a post etc., no need to load these 17 files on every page.
Expected Behavior
Make sure that only files are loading that used on the requested page.
Environment
The text was updated successfully, but these errors were encountered: