search

FortAwesome / wordpress-fontawesome

Font Awesome Official WordPress Plugin
Other
57 stars 19 forks source link

Optimization: avoid autoloading all release metadata on every page load #51

Closed mlwilkerson closed 3 years ago

mlwilkerson commented 5 years ago

See my comments on this support forum topic.

Currently, the release provider metadata is stored in the non-expiring transient named after FontAwesome_Release_Provider::RELEASES_TRANSIENT. This is to avoid making a network request for the release metadata on each page load. However, it's not necessary to load all of this metadata on each page. Once a particular release is made active, it would suffice for only that one release's metadata to be loaded on each page load.

So one approach could be to store the currently active release metadata separately for normal page loads. The full release metadata could be loaded only when loading the admin settings page.

All of this may change, depending on how this plugin enables kits in the future. That is because the releases metadata in question is largely required for loading Font Awesome from the non-kits CDNs. If when adding kits support we decide to drop non-kits CDN support, this releases metadata may become obsolete altogether.

SJNBham commented 4 years ago

Any update on this?

mlwilkerson commented 4 years ago

@SJNBham Thanks for asking. I had to take another close look to see if there were any updates to report.

In nutshell, I did add kits support, and there are no plans to remove the non-kits CDN support. That work did not end up involving any significant changes to how the CDN releases metadata are loaded. So I do think this is still an outstanding issue.

After some fresh analysis, here are some implementation notes to my future self, or possibly another contributor who'd like to take a crack at it:

  1. We should probably set the RELEASES_TRANSIENT_EXPIRY to something large but still expiring (eventually, even if far into the future--like a year).

    That's because transients that never expire are always autoloaded.

    This would stop the autoloading from happening, but still--if on every page load, it still needs to be loaded (even if not auto loaded), then--we wouldn't have saved much.

  2. The place where on a normal page load the relevant metadata need to be retrieved is here.

    I haven't confirmed it by experiment yet, but based on reading that area of the code, it does look like configuring the plugin with "Use a Kit" would result in not ever calling into the ReleaseProvider, and therefore never needing to trigger a load of this stored metadata.

    So if we added the expiration time to the transient AND the plugin were configured to use a kit, then I think this metadata would stop loading at all on a normal page load. So when no releases metadata are required, none would be loaded.

  3. The next level would be to avoid loading all of the releases metadata when only some of it is needed, as when the plugin is configured with "Use CDN."

    That could be accomplished by adding an additional option in the db that stores the release metadata for the currently configured version.

    So when FontAwesome::init() needs to get the CDN resource metadata for the currently configured version, it would either first check to see if there's already one that's been stored in that separate option, or it would delegate that responsibility of storing and keeping track of that separate option to the ReleaseProvider.

    In terms of separation of concerns and/or encapsulation, the FontAwesome class is the one that knows what's currently configured. The ReleaseProvider is the one that knows how to provide access to the CDN releases metadata. So it might make the most sense for the FontAwesome class to be the one that manages this extra option for stashing the CDN resource metadata for the currently configured version of Font Awesome.

    Another possibility would be, rather than storing the current CDN release metadata in a separate option in the db, store it with the currently configured version in the existing option. This keeps data together that should only ever change together, so there would be less likelihood for data inconsistencies. A possible downside (but probably not too bad) is that it changes the schema of that option again, so it would also be necessary to handle plugin version upgrades appropriately. (What about downgrades?)