-
Notifications
You must be signed in to change notification settings - Fork 168
feat: improving page load time #3605
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
base: main
Are you sure you want to change the base?
Conversation
@rithikb24 has imported this pull request. If you are a Meta employee, you can view this in D82455607. |
Hi @rithikb24, I am providing you with the results I got from testing this feature.
Based on these results, the observed differences are negligible and fall within the range of normal variation between requests. I am also attaching a document containing the full set of logged requests along with the computed averages for both scenarios. |
Hi @ukilla - could you share how you used query monitor plugin to get these metrics? |
Hi @rithikb24 - I have used Query Monitor to write down the performance metrics of Home, Shop and Product pages. |
Thanks for sharing - One more thing, I noticed a consistent 100ms+ improvement using Code Profiler plugin to check plugin execution time. This issue suggested this improvement - #2878 |
@ukilla I think the 100ms+ performance gain from composer autoloader happens before query monitor even starts measuring page load time. |
@rithikb24 After reviewing the implementations of Code Profiler and Query Monitor, I confirmed that Query Monitor initializes on the Because of this difference in load order, Query Monitor will not capture performance gains that occur during the bootstrap phase (such as improvements from delaying the Composer autoloader), while Code Profiler does. |
@ukilla Yep! I came to the exact same conclusion - thank you for confirming! |
@rithikb24 - Could you share how you used Code Profiler to test the execution times? On our tests it seems like the execution time increased with the plugin version that has this fix. We tested the Home, Shop, Cart and Product pages. |
…TOLOADER_LOADED to prevent fatal errors when Checker class is already loaded by another plugin
Hi @rithikb24, We were able to simulate a fatal error using a MU plugin that preloads the Checker class. This confirmed that the previous approach of delaying Composer loading based solely on In the issue you mentioned, the developers of other plugins solved the problem by requiring the Composer autoloader inside the functions that actually need Composer classes. Since we need the Composer classes in init_plugin(), your original approach of including the autoloader there is appropriate. I added a |
Hi @ukilla - really appreciate the detailed input here - thanks for adding the changes. I will take a look! |
Description
Moving autoloader only when the plugin is initialized - improving memory usage and plugin performance
Type of change
Checklist
Changelog entry
Moving autoloader only when the plugin is initialized - improving memory usage and plugin performance
Test Plan
Screenshots
Please provide screenshots or snapshots of the system/state both before and after implementing the changes, if appropriate
Before
After