-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Lazy load uncommon CFG_GLPI values #21528
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: 11.0/bugfixes
Are you sure you want to change the base?
Lazy load uncommon CFG_GLPI values #21528
Conversation
|
Please fix lint |
| $.ajax({ | ||
| type: 'GET', | ||
| url: CFG_GLPI.root_doc + '/Session/Config/' + prop, | ||
| async: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed on similar changes, we might disagree on this but I am against adding synchronous calls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't disagree on this point. If it is async though, something will try using a config value that isn't loaded, trigger it to be loaded, and immediately get no value since it didn't wait for the response. I don't want it to be synchronous but it needs to be to make this work with existing code.
We could close the PR and leave the entire dump available, but I'm just trying to improve performance as best I can with what we have for now.
Checklist before requesting a review
Description
One of a few possible optimizations to improve GLPI performance. I only consider this a short-term optimization when there are better ways to handle this and it is not a substantial optimization on its own.
Dumping the entire safe CFG_GLPI array in a JS script on the page is excessive.
We only really use the "root_doc" config in most places.
Even in plugins as far as I can see, there are very few non-"root_doc" uses of this variable.
Instead of dumping the entire config, I replaced it with a proxy to transparently lazy-load missing values.
This would be a synchronous call but it was necessary to not break existing code and the need to fetch additional configs should only rarely come up.
I believe the faster initial page loads would be worth it.
Tested in production env.
Seeing approximately 8kb (compressed size) less transferred per page and approximately 30ms less page download times.
Seeing "Time to DOM interactive" and "Time to DOM complete" metrics a 100-200ms lower on average but this can vary a lot.
Minor performance improvements and data usage reductions should be seen across the entire app for initial page loads.