added bulk read support

[matej21]

• is fallback necessary ?
• SQLiteStorage and NewMemcachedStorage: read methods internally use bulkRead. Is it ok? I think performance impact won't be serious.
• non-scalar keys are not allowed in a bulkLoad - alternative would be serialize non-scalar keys. what do you think?

[Majkl578]

Does it make sense to have one fallback? Woulldn't make more sense to have fallback for each key? Or support both?

[matej21]

key is passed as a first parameter to the fallback.

[Majkl578]

Yes but then you can't easily reuse callback for load and bulkLoad.

[matej21]

So each time you call bulkLoad you will build an array of callbacks? that does not sound right.

[Majkl578]

What if you want e.g. three values whose value are calculated in a different way? You would have to build this logic by if/switch branching inside the callback. That feels a bit ugly...

[enumag]

I agree with @matej21.

@Majkl578 How about this?

public static function createCallbackSwitcher(array $callbacks)
{
    return function ($key, ...$args) use ($callbacks) {
        return $callbacks[$key](...$args);
    };
}

$cache->bulkLoad($keys, self::createCallbackSwitcher($callbacks));

[Majkl578]

I don't think bulk reuse for single operation is a good idea, for performance reasons.

[JanTvrdik]

Replaces whole line with

$storageKeys = array_map([$this, 'generateKey'], $keys);

[Majkl578]

Have you seen how doctrine/cache handles this? https://github.com/doctrine/cache/blob/master/lib/Doctrine/Common/Cache/CacheProvider.php
I like that approach more, they implement both, but for drivers that don't support multi-get, they fall back to serial fetch.

[JanTvrdik]

TODO: consider returning [$value1, $value2] instead of more complex [$key1 => $value1, $key2 => $value2]

[matej21]

Storage would have to maintain the item order and check for missing values. This is easier.

[JanTvrdik]

Yes but the implementation in Cache would be simpler and faster.

[matej21]

Yes but the same logic would have to be in the storage itself.

[JanTvrdik]

That would depend on the storage. Now you assume that no storage can implement this efficiently.

[matej21]

Yes, I do assume that. Or do you know any storage that doesn't need that logic?

[JanTvrdik]

The ideas is awesome, but the implementation needs a lot polishing.

[JanTvrdik]

I think dg uses test(function () { // storage without bulk load support pattern

[dg]

It does not matter :-)

[matej21]

btw, the performance impact of bulkRead reuse for a single read is about 10-20%

[JanTvrdik]

Drop this line, replace all usages of $storage with $this->storage

[matej21]

updated - storages are optimized

[JanTvrdik]

Replace with

$prefixedKeys = array_map(function ($key) { return urlencode($this->prefix . $key); });
$keys = array_combine($prefixedKeys, $keys); // TODO: can we remove this?
$metas = $this->memcached->getMulti($prefixedKeys);