Introduce wp_cache_get_multi()

[nacin]

Both options (see #10274) and themes (see #20103) could benefit from a cache backend that implements get_multi(). For options, this means we can use individual keys. For both themes and options, we'd be able to make fast multiple gets.

Both APC and Memcached (but not Memcache) implement multiple-get. A fallback would be looping over get().

Separately, as this would be a new function we use in core, we probably need to start doing some kind of versioning for drop-ins like db.php and object-cache.php.

[tollmanz]

I imagine that there is a lot to be discussed about how this should work, but I am adding a patch to move things along. I added a first attempt at a get_multi function with this patch. Additionally, I have written unit tests and uploaded them here (would appreciate some direction on if these should also be added to the unit test trac and what's the right way to do that organizationally).

This get_multi function uses the get function to retrieve values from the $cache var. An array of keys and groups are sent to the function. The function attempts to figure out the relationship between the $keys and $groups values. The logic is:

• If $keys and $groups are both strings, it is assumed to be a simple get.
• If $keys and $groups are arrays and are the same length, $keys[n] is assumed to be in $groups[n].
• If there are more $groups than $keys, $keys[n] is assumed to be in $groups[n] until all values in $groups is exhausted. At that point, the remaining $keys values are assumed to belong to the group 'default'.
• If $keys is an array and $groups is a string or single value array, all $keys are assumed to be in $groups (if string) or $groups[0] (if array).

Values are returned as an array with top level keys being groups. The second level keys are the cache key values. This should match the organization within the $cache var itself.

A number of different scenarios have been tested in the unit tests and this seems to be working well for the assumptions I have laid out.

[wonderboymusic]

I cribbed the Memcache code to implement it here: ​http://wordpress.org/extend/plugins/memcached-redux/

[ryan]

For back compat reasons, the API here should handle the call signature used in the memcached plugin. Looks like the patch might already handle that. ​http://plugins.svn.wordpress.org/memcached/trunk/object-cache.php

[wonderboymusic]

I squeezed the code into new code - I don't think the return value needs to be associative and bucketed. I think a standard use-case for get_multi in action is:

list( $one, $two, $three ) = wp_cache_get_multi( array( 'one', 'two', 'three' ), 'numbers' );

[wonderboymusic]

these are all marked 3.7-early

[nacin]

Holding off on this until ryan can weigh in. We don't have an acute need for it in core.

[chriscct7]

Lack of interest in 2 years. Closing as maybelater. Feel free to reopen if interest re-emerges and there is a demonstrable need for it in core (per comment:9)

[jeremyfelt]

While there hasn't been an immediate need for multi-get, I don't think it's a non-interest yet. While we may not end up using it in core directly, having it as part of the API could be beneficial to developers.

[spacedmonkey]

+1 to what @jeremyfelt said. I know a number of plugins already using get multi. Some of these are plugins provided my Automattic. Having to write logic to check if get multi exists is a pain. It would be a big win for me if this goes in, as I don't have write those checks anymore and forces object cache implementations to have these functions available.

[spacedmonkey]

Example of it in use in the wild

​https://github.com/Automattic/advanced-post-cache/blob/4f59ea7ca4e202363804601589a2e4ca3a3ee71c/advanced-post-cache.php#L128

And others have already implemented it

​https://github.com/ericmann/Redis-Object-Cache/blob/master/object-cache.php#L120

And of course

​http://wordpress.org/extend/plugins/memcached-redux/
Which is becoming popular.

There is a desire for this

[boonebgorges]

#36953 is a concrete use case for wp_cache_get_multi() in core.

How can we move this ticket forward? Since it looks like plugins and cache drop-ins are already implementing wp_cache_get_multi(), maybe a good place to start is to do an inventory to understand what the interface has got to look like. I'm not sure how wp_cache_get_multi( $keys, $groups ) would work, since individual keys are group-specific. ​https://github.com/ericmann/Redis-Object-Cache/blob/master/object-cache.php#L120 seems like it has to be the correct function signature.

It would be helpful to get developers of some popular cache plugins looped into the conversation, so that we can all be on the same page with function signatures, etc.

[spacedmonkey]

Related: #31245

[boonebgorges]

A couple of patches, in the interest of moving this ticket along.

20875.3.diff​ is a core patch that shows what the default wp_cache_get_multi() might look like. Instead of the ( $keys, $groups ) syntax suggested by @tollmanz, I've gone for the more straightforward multidimensional array:

$cached = wp_cache_get_multi( array(
    'cache_group_1' => array( 'foo', 'bar' ),
    'cache_group_2' => array( 'baz' ),
) );

This syntax is more verbose in many cases, but (a) it's more consistent and thus clearer, and (b) it better parallels the format of the array returned by the function. This format is what's expected by ​https://github.com/ericmann/Redis-Object-Cache/blob/master/object-cache.php#L546 and ​https://plugins.svn.wordpress.org/memcached/trunk/object-cache.php, but different from what's expected by ​https://github.com/mgmartel/memcached-redux/blob/master/object-cache.php#L256 and ​https://github.com/tollmanz/wordpress-pecl-memcached-object-cache/blob/master/object-cache.php#L1384 (the latter two expect @tollmanz's syntax, or some subset of it). We are going to have to break compatibility with something, so I'd suggest we choose the syntax we like the best. I prefer what's in my patch, but I don't feel strongly about it.

20875-example-usage.diff​ is an example of how it'd be used in core. The example is from #36953. This is a particularly interesting example because it's fetching from lots of cache *groups* at the same time. (If we go forward with this ticket, I'd move the logic from this patch into get_object_term_cache() - the patch is for demonstration only.)

20875-implementation-apc.php​ is a totally untested PoC patch for the APC drop-in.

I haven't accounted for the $force or $found parameters that get() has. It's hard to deal with them without making the syntax even more convoluted. Worth noting that the Redis and Memcached plugins listed above also do not support these flags, for get_multi() *or* for get().

It's still #yolofriday where I live, so I'm putting this up for 4.6 consideration.

[spacedmonkey]

I think it is important this is in core ASAP. Third party plugins have supported get multi for years and other plugin providers already started using the function. Example is this.

​https://github.com/Automattic/advanced-post-cache/blob/master/advanced-post-cache.php#L128

Get multi needs to be in core for the simple reason that a consistent api needed in place to stop future issues with plugins like the above.

[dd32]

cross-linking to #31245 which has a lot of input on multi_get(), https://core.trac.wordpress.org/ticket/31245#comment:6 is an example of how splintered the current ecosystem is.

I'm hesitant to comment here, as I think it should be handled in #31245 simply due to the amount of discussion there (or someone should summarize that ticket here, and direct all further discussion here).

I think the only route forward here will be to find a new name, as the wp_cache_get_multi() function signature is just not viable thanks to existing users (even the ones which have insane return values)
wp_cache_multi_get()? wp_multi_cache_get()? wp_cache_get_multiple()? wp_cache_getS()?

[boonebgorges]

Thanks, @dd32. There is lots of useful discussion in #31245, but that ticket is covering two separate issues: (a) the implementation of get_multi(), and (b) whether and how get_multi() can help to solve the alloptions problem. Because we're developing good use cases for get_multi() outside of alloptions, I would hate to see the Cache API improvement held up because we can't come to a consensus about alloptions.

I agree that going with a different function name probably makes sense, given the compatibility concerns. wp_cache_get_multiple() seems best to me.

In https://core.trac.wordpress.org/ticket/31245#comment:3, @rmccue asks:

I'm not entirely sure on the _wp_cache_compat_get_multi naming/usage pattern here, but doing it in a compatibility file requires us to either define the function inside wp_start_object_cache or in a new file after wp_start_object_cache but before default-filters.php is loaded. Thoughts?

This compatibility pattern seems bad to me because it means that we need to a function_exists() check every time we want to use the new function. I suggest loading a compatibility file *inside of* wp_start_object_cache(). We do this already, when object-cache.php is not found. 20875.4.diff​ does this; it also switches to wp_cache_get_multiple(), uses @rmccue's variable names instead of my confusing ones, and simplifies by setting values not found in the cache to be set to false (rather than leaving them out of the array altogether), which makes the code simpler and is also closer to expected behavior, IMO.

[spacedmonkey]

Another really good use for get multi would be in _get_non_cached_ids function. Currently this loops around all the ids it gets out of cache. For performance it would be much better to do this in one, if possible.

I like the idea of calling the function wp_cache_get_multiple. It stops issues with existing object-caching drop-ins.

[rmccue]

I'd like to get feedback from @tollmanz and @tellyworth on this one, as it affects a lot of caches and affects WP.com too. We may need to introduce this without using it for alloptions in #31245 if performance of alloptions is going to be a large concern.

Definitely agreed that alloptions/multi-get shouldn't block each other. alloptions is a very specific use case for multi-get that doesn't necessarily fit the typical use case for it; for most uses, an iterated single get (i.e. the fallback) isn't hugely different.

[boonebgorges]

Replying to rmccue:

I'd like to get feedback from @tollmanz and @tellyworth on this one, as it affects a lot of caches and affects WP.com too. We may need to introduce this without using it for alloptions in #31245 if performance of alloptions is going to be a large concern.

Definitely agreed that alloptions/multi-get shouldn't block each other. alloptions is a very specific use case for multi-get that doesn't necessarily fit the typical use case for it; for most uses, an iterated single get (i.e. the fallback) isn't hugely different.

Agree 100%, especially about the last part. Functions like _get_non_cached_ids() already do a bunch of separate _get() calls. Switching to use getMulti() when available will do no harm at all in cases when the object cache doesn't implement it.

I think the main outstanding questions here have to do with function naming and syntax. On this point, @tollmanz's thoughts would be especially helpful, since he originally suggested (and implemented in his own cache backends) the more sophisticated syntax.

[ocean90]

This seems to require some feedback by authors of the leading cache drop-ins. Punting.

[jipmoors]

Wanted to throw in another variation for the naming: wp_cache_get_bulk

[boonebgorges]

See #37762. The cache_results parameter of WP_Query was introduced to skirt problems with bulk cache set/get, but the parameter is mostly broken. Once we have these bulk cache functions, we may want to remove (or otherwise fix) cache_results.

[johnjamesjacoby]

This ticket is relevant to my interests.

I followed Memcached's approach in ​WP Spider Cache, and it feels the most WordPress-like in it's signature to me.

[boonebgorges]

Replying to johnjamesjacoby:

This ticket is relevant to my interests.

I followed Memcached's approach in ​WP Spider Cache, and it feels the most WordPress-like in it's signature to me.

IMO, it feels WordPress-like in all the worst ways (doing one magic thing with a string $groups, and a different magic thing with an array), but as noted in 16, my feelings aren't particularly strong one direction or another.

To move this forward, someone needs to make the call about syntax and function naming. Authors of existing drop-ins (excepting @johnjamesjacoby) don't seem eager to wade into this discussion, so I think we need an executive decision. I nominate @dd32 to make that decision, unless he wants to delegate his authority to me ;)

[dd32]

To move this forward, someone needs to make the call about syntax and function naming. Authors of existing drop-ins (excepting @johnjamesjacoby) don't seem eager to wade into this discussion, so I think we need an executive decision. I nominate dd32 to make that decision,

I think we just need to choose the most sane function signature/return values and go with it - but with the caveat that we can't use an existing name if a common dropin has a conflicting method signature.

20875.4.diff​ seems sane to me, to recap it:

• wp_cache_get_multiple( $groups ) which doesn't exist in the wild that I can see - it's within the wp_cache_ namespace, so if something private has used it, good luck to them.
• Signature of $groups as array( group1 => [ key1, key2 ], group2 => [ key3 ] ); IMHO looks sane and balances the complexity of fetching from multiple groups while at the same time catering to it.
• Return signature of array( group1 => [ key1 => value1, key2 => value2 ], group2 => [ key3 => value3 ] ) matches the input signature, which makes usability far more consistent.

Looking at real world use-cases:

• $posts = wp_cache_get_multiple( [ 'posts' => [ 1, 2, 3 ] ] )['posts'];
• $cache_data = wp_cache_get_multiple( [ 'posts' => [ 1 ], 'postmeta' => [ 1 ] ); $post = $cache_data['posts'][1]; $post_meta = $cache_data['postmeta'][1];
• $option_names = [ 'option_1', 'option_2' ]; $options = wp_cache_get_multiple( [ 'options' => $option_names ] );

Moving forward on this ticket:

• I'd like to see the authors of the various object caches to be consulted, with 20875.4.diff​ as the suggested core patch (I don't see a compiled list of object caches and their authors here)
• A quick yay-nay from each and maybe examples of how the cache would implement it, to ensure that there's no unexpected surprises for authors. (Such as the APC one in 20875-implementation-apc.php​). This could be done by others as patches or PRs to projects too (Best to ask no cache to implement the signature until core has a commit though!)
• A few examples of cases in WordPress where it can be utilised immediately (through the compat wrapper) which shows it's beneficial and actually offers improvements when implemented. (Such as done in 20875-example-usage.diff​

From there, I think we're surprisingly in a good position to move forward here.

[spacedmonkey]

Good to see progress on this ticket. Regarding getting feedback for drop-in authors, we can reach out to some of them on slack and see what they say. I know that many of the most interested parties have already set this ticket to watched anyway. But I am worried that if this is left hanging anymore it will get held up. Maybe we could set a hard deadline for feedback of say, 4 weeks. If we don't hear back from them in this time, just go a head. We can keep backwards compatibility by using function_exists in the right places.

A reviewed the core for good places to implement a multi get and found these functions.

update_meta_cache
update_object_term_cache
_get_non_cached_ids

All of these a simple for each loops from one group, so shouldn't be too painful to implement.

[spacedmonkey]

20875.5.diff is an update to the patch.

A small change to the compat fix.

I would really like to see this in core. It has been ages, the object-cache drop-in plugin developers, have not feedback, let's move forward wp_get_cache_multiple, as it is not a breaking change.

[spacedmonkey]

See attached to this ticket, three example implementation.

• update_meta_cache
• update_object_term_cache
• _get_non_cached_ids

When this gones in, there should become there own tickets.

[tillkruess]

@tellyworth @SergeyBiryukov Would you guys mind having a look at this? It's been idle for 2 years but would greatly improve performance.

[spacedmonkey]

I have changed the patch so that format is different.

In 20875.7.diff there are the following changes.

• Function name change changed to wp_cache_multiple_get. This isn't the best english, but it follows the pattern of having get the end like other functions like wp_cache_get and wp_remote_get.
• Much simpler to understand api. Give a list of keys, single group. This means a developer can only get keys from one group at at time. But in examples of usable in core update_meta_cache,update_object_term_cache, _get_non_cached_ids, only one cache group is being request anyway.
• Added force param, to bring inline with wp_cache_get

[donmhico]

Thanks for all the patch. In 20875.8.diff, I fixed the failing unit test by adding the wp_cache_multiple_get in signature check.

[davidbaumwald]

Latest patch needs to be refreshed since [47197].

[tillkruess]

I've updated the patch to match [47197].

I also renamed the wp_cache_multiple_get() to wp_cache_get_multiple(), since it's called WP_Object_Cache::get_multiple() and easier to read/remember.

[tillkruess]

fixed method name typo in test method

[davidbaumwald]

@SergeyBiryukov Does the refreshed patch look good to you?

[SergeyBiryukov]

20875.10.diff​ refers to ABSPATH . WPINC . '/cache-compat.php' which doesn't exist in core.

Apparently that file is added by an earlier 20875.5.diff​ patch, but the function signature there is different and could use better docs. That should be easy to fix, but I'm also not sure it requires a separate file. There's some history here that I'd like to go through.

As much as I'd love to get this in, let's move to early 5.5 to make sure everything is correct.

[tillkruess]

I'm sorry, that was sloppy. I updated the patch:

• Fixed comment indentation in unit test
• Updated version to 5.5
• Moved loading of cache compat file in wp_start_object_cache() (removed condition)
• Added missing cache-compat.php file

Having a wp_cache_get_multiple() compat fallback (when a 3rd party object cache is used) will allow WP core to use that function without causing a fatal errors.

[whyisjake]

In 47938:

Cache API: Introduce wp_cache_get_multi().

Many caching backend have support for multiple gets in a single request. This brings that support to core, with a compatability fallback that will loop over requests if needed.

Fixes: #20875.
Props: nacin, tollmanz, wonderboymusic, ryan, jeremyfelt, spacedmonkey, boonebgorges, dd32, rmccue, ocean90, jipmoors, johnjamesjacoby, tillkruess, donmhico, davidbaumwald, SergeyBiryukov, whyisjake.

[desrosj]

This should probably receive a dev note. Reopening and marking as such.

[whyisjake]

See also [47939] which brought the cache-compat.php file that was missed in the initial commit.

[desrosj]

In 47942:

Coding Standards: Cleans up some PHPCS issues introduced in [47938].

See #20875.

[spacedmonkey]

@whyisjake I have created a follow up ticket to use this new function in core. See #50352. It even has a patch.

[SergeyBiryukov]

In 47944:

Docs: Add missing documentation for the $group parameter of WP_Object_Cache::get_multiple().

Synchronize documentation between wp_cache_get_multiple(), its compat version, and the class method.

See #20875.

[desrosj]

Going to close this out to help clear the milestone of completed tickets. I've confirmed this is on the list of dev notes being tracked for 5.5, so it won't get lost in the shuffle. If anyone feels particularly interested in drafting the note, just reach out on Slack.