wp-bootstrap / wp-bootstrap-navwalker

A custom WordPress nav walker class to fully implement the Twitter Bootstrap 4.0+ navigation style (v3-branch available for Bootstrap 3) in a custom theme using the WordPress built in menu manager.
https://wp-bootstrap.github.io/wp-bootstrap-navwalker/
GNU General Public License v3.0
3.37k stars 1.94k forks source link

Object Cache Support #362

Open bhubbard opened 6 years ago

bhubbard commented 6 years ago

@pattonwebz @sfgarza

Was looking to make some improvements for a client site, went back to thinking about the menu improvements. I threw together a quick branch where I added WP Object Cache support. Seems to be working but would like to get more eyes on it, if you can take a look when you get the chance.

https://github.com/wp-bootstrap/wp-bootstrap-navwalker/tree/object-cache

pattonwebz commented 6 years ago

I can take a look at it in the next few days and test it out. Quick scan of the diff looks ok though :+1:.

Is this essentially the implementation you went with on the client site? And is it working all good on there?

pattonwebz commented 6 years ago

Hey, so I tested this out in my staging server and it's not quite working correctly.

When objects are being set they are using a $group set as wp-bootstrap-navwalker but when getting they are not using the key. Cached items are never retrieved and you can check that if you enable object caching with debug on in W3 Total Cache by seeing these entries:

  911 |  get  |       not in cache        |               |         0.0001 |         default | wp_bootstrap_navwalker_item_output
  912 |  set  |       put in cache        |           168 |              0 | wp-bootstrap-navwalker | wp_bootstrap_navwalker_item_output
  913 |  get  |       not in cache        |               |         0.0001 |         default | wp_bootstrap_navwalker_item_output
  914 |  set  |       put in cache        |           131 |              0 | wp-bootstrap-navwalker | wp_bootstrap_navwalker_item_output
  915 |  get  |       not in cache        |               |         0.0001 |         default | wp_bootstrap_navwalker_item_output
  916 |  get  |       not in cache        |               |         0.0001 |         default | wp_bootstrap_navwalker_linkmod_type
  917 |  set  |       put in cache        |            24 |              0 | wp-bootstrap-navwalker | wp_bootstrap_navwalker_linkmod_type
  918 |  get  |       not in cache        |               |         0.0001 |         default | wp_bootstrap_navwalker_eopen_output
  919 |  set  |       put in cache        |            93 |              0 | wp-bootstrap-navwalker | wp_bootstrap_navwalker_eopen_output
  920 |  get  |       not in cache        |               |         0.0001 |         default | wp_bootstrap_navwalker_eclose_output
  921 |  set  |       put in cache        |            13 |              0 | wp-bootstrap-navwalker | wp_bootstrap_navwalker_eclose_output
  922 |  set  |       put in cache        |           106 |              0 | wp-bootstrap-navwalker | wp_bootstrap_navwalker_item_output
  923 |  get  |       not in cache        |               |         0.0001 |         default | wp_bootstrap_navwalker_item_output
  924 |  get  |       not in cache        |               |         0.0001 |         default | wp_bootstrap_navwalker_linkmod_type
  925 |  set  |       put in cache        |            23 |              0 | wp-bootstrap-navwalker | wp_bootstrap_navwalker_linkmod_type
  926 |  get  |       not in cache        |               |         0.0001 |         default | wp_bootstrap_navwalker_eopen_output
  927 |  set  |       put in cache        |           104 |              0 | wp-bootstrap-navwalker | wp_bootstrap_navwalker_eopen_output
  928 |  get  |       not in cache        |               |         0.0001 |         default | wp_bootstrap_navwalker_eclose_output
  929 |  set  |       put in cache        |            14 |              0 | wp-bootstrap-navwalker | wp_bootstrap_navwalker_eclose_output
  930 |  set  |       put in cache        |           128 |              0 | wp-bootstrap-navwalker | wp_bootstrap_navwalker_item_output
  931 |  get  |       not in cache        |               |         0.0001 |         default | wp_bootstrap_navwalker_item_output
  932 |  set  |       put in cache        |           163 |              0 | wp-bootstrap-navwalker | wp_bootstrap_navwalker_item_output
  933 |  get  |       not in cache        |               |         0.0001 |         default | wp_bootstrap_navwalker_item_output
  934 |  set  |       put in cache        |            86 |              0 | wp-bootstrap-navwalker | wp_bootstrap_navwalker_item_output
  935 |  get  |       not in cache        |               |         0.0001 |         default | wp_bootstrap_navwalker_item_output
  936 |  set  |       put in cache        |           136 |              0 | wp-bootstrap-navwalker | wp_bootstrap_navwalker_item_output
  937 |  get  |       not in cache        |               |         0.0001 |         default | wp_bootstrap_navwalker_item_output
  938 |  get  |       not in cache        |               |         0.0001 |         default | wp_bootstrap_navwalker_sr_text
  939 |  set  |       put in cache        |            37 |              0 | wp-bootstrap-navwalker | wp_bootstrap_navwalker_sr_text
  940 |  set  |       put in cache        |           169 |              0 | wp-bootstrap-navwalker | wp_bootstrap_navwalker_item_output
  941 |  get  |       not in cache        |               |         0.0001 |         default | wp_bootstrap_navwalker_item_output
  942 |  set  |       put in cache        |           104 |              0 | wp-bootstrap-navwalker | wp_bootstrap_navwalker_item_output

The 2nd issue is that only 1 menu item is ever stored in the cache for each key. Every wp_cache_set overrides the previous one. When the group is added to the wp_cache_get all items that output become the same item as the last one that would have been cached. Screenshots may help explain:

image ^ With cache enabled. image ^ Without cache (expected output).

pattonwebz commented 6 years ago

I think that it would be a very useful feature to add deep level caching to the walker and I have experimented in the past but found it quite tricky (and many methods quite inefficient with the amount of things that need stored).

I've opted for page level caching of the full menu markup (by wrapping the wp_nav_menu call inside the template file and appending the post_type + slug to the cache key) instead of doing it inside of the walker on an item by item basis.