wp-bootstrap-navwalker icon indicating copy to clipboard operation
wp-bootstrap-navwalker copied to clipboard

Object Cache Support

Open bhubbard opened this issue 7 years ago • 3 comments

@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

bhubbard avatar Mar 30 '18 17:03 bhubbard

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 avatar Mar 30 '18 17:03 pattonwebz

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 avatar Mar 30 '18 19:03 pattonwebz

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.

pattonwebz avatar Mar 30 '18 20:03 pattonwebz