Add $found parameter to wp_cache_get_multiple()

[peterwilsoncc]

Combination of patches on trac ticket WP#57743

[spacedmonkey]

@peterwilsoncc Do memcache and redis support this?

[peterwilsoncc]

@peterwilsoncc Do memcache and redis support this?

Memcache does, I've updated the implementation in the test suite to do so.

@tillkruess do you know if redis supports it?

[peterwilsoncc]

Sorry, wrong username, @tillkruss do you know if redis supports passing found by reference when getting multiple objects?

[tillkruss]

Yes, it could be added to the plugins. But Litespeed, W3TC and others are pretty stale and won't support this for a long time.

[tillkruss]

Does this serve a purpose in core?

How could this be used in the real world, isn't the false value already giving us this information?

[peterwilsoncc]

Does this serve a purpose in core?

Mainly, it makes the getter API for single and multiple cache objects consistent.

How could this be used in the real world, isn't the false value already giving us this information?

Often but not always, the real world implication is when the cached object is false. In the example below, the slow function will be run for foo even when the result is cached.

Core caches false values in some places but not many. In some circumstances, it's quite common that a false needs to be cached.

function pw_slow_function( $result ) {
  sleep( 2 );
  return $result;
}

wp_cache_set( 'foo', pw_slow_function( false ) );
wp_cache_set( 'bar', pw_slow_function( true ) );

$values = wp_cache_get_multiple( [ 'foo', bar' ] );

foreach ( $values as $key => $values ) {
   if ( ! $value ) {
      $value = pw_slow_function( /* as above */ );
      wp_cache_set( $key, $value );
   }
}

[tillkruss]

The consistency makes sense.

Just for public record, storing falsy values is always risky and often leads to silent errors.

[tillkruss]

I don't think Memcached and Redis can reliably support this, because if a user stores false or null as a cache value, we don't know whether the result is false or if we didn't get a cache value.

Checking for existence first doubles to lookup. It's like running file_exists() and file_get_contents() which is possible is two operations per key, not one.

Typically WordPress expects non existing cache keys to return false, so storing false values won't allow us to distinguish between these.

I'm not sure this PR makes sense, the root issue is what the user code does.

[peterwilsoncc]

I'll investigate further to make sure it's working as expected.

For ::get(), the WP persistent cache implementation checks the result code and defines $found based on that:

wordpress-develop/tests/phpunit/includes/object-cache.php

Lines 1468 to 1471 in 464ad20

	if ( Memcached::RES_SUCCESS === $this->getResultCode() ) {
	$this->add_to_internal_cache( $derived_key, $value );
	$found = true;
	}

For get multi, maybe it's possible to use array_key_exists() but I'll need to add some tests.

Does Redis have an equivalent, I notice the docs reference the return value (nil) for non-existent keys but I'm unclear if that's a magic string or a return code.

[spacedmonkey]

I do think that redis could support this functionality. See this method. It might tricky, but I see how it could be done.

Consider that I am really struggling to get cache plugins even support wp_cache_get_multiple.

https://wordpress.org/support/topic/implement-function-wp_cache_get_multiple/
tlovett1/simple-cache#144
humanmade/wp-redis-predis-client#12
litespeedtech/lscache_wp#259
BoldGrid/w3-total-cache#239

I have more this ^

[tillkruss]

I do think that redis could support this functionality. See this method. It might tricky, but I see how it could be done.

That's just using Redis's MGET which will return false if a key doesn't exist in a GET or MGET call. Predis is a bit better and returns null for non-existent keys.

But it's just ambitious to store falsy values in the cache.

I have more this ^

😞