once should use the parameters of the closure to generate the once-per-combination key (PHP 8.1)

[Sairahcaz]

Hi @freekmurze,

I love the once package.
It really helped me on some performance issues in a legacy and spaghetti codebase.

But I noticed, that in some circumstances, once is not working as I was expecting it to do.

Let's take a look at the following example:

    public function processSomethingHeavy($user)
    {
        // ...

        $relationId = $user->foo_relation_id;
        $relation = once(function () use ($relationId) {
            return Foo::find($relationId);
        });
        
        // ...
    }

I thought I was fine with this, but then I saw, that once is creating the once-per-combination key by the arguments provided to the outer function. That means, if I call the processSomethingHeavy on 10000 users, of which a high percentage may have the same foo_relation_id, but all of them for sure have a different user_id, the cache will not work, cause the once-per-combination key gets generated by the outer function parameters. With this I would drastically lose performance here!

Since PHP 8.1 you can dynamically get the parameters passed to a callback like this:

$callbackFunc = new \ReflectionFunction($callback);
$closureArguments = $callbackFunc->getClosureUsedVariables();

With that code we could generate the once-per-combination key from the parameters passed to the closure, which I think is much more intuitive and also as shown above would lead to a better cache hit and performance!

Cause I needed it fast, I wrote a own small memoization package.
But if you think about implementing this approach, I would definitly use the once package again!

[Sairahcaz]

Bump ☺️

[solodkiy]

Maybe you should try another lib. This one works quite like you expecting: https://github.com/solodkiy/memorize

[freekmurze]

Since there might be many people relying on the current behaviour, I'm going to leave things as they are for now.

[henzeb]

came across similar myself today. Perhaps it's a good idea to note this in the documentation? or add a flag/extra function? although with once coming natively to Laravel 11, this might be a change you want also to be adopted there.

As a workaround, people can add the following to a Function helper file in their projects. The body is the same as the once function, but generates a md5 hash based on just the spl_object_hash and functionName. It isn't as thoroughly as the $backtrace->getObjectHash(), but it does the trick for me. if you need it to be unique using those used $variables, you could use the snippet from @Sairahcaz to generate the hash.

use Spatie\Once\Backtrace;
use Spatie\Once\Cache;

if (!function_exists('only_once')) {
    function only_once(callable $callback): mixed
    {
        $trace = debug_backtrace(
            DEBUG_BACKTRACE_PROVIDE_OBJECT, 2
        );

        $backtrace = new Backtrace($trace);

        if ($backtrace->getFunctionName() === 'eval') {
            return call_user_func($callback);
        }

        $object = $backtrace->getObject();

        $cache = Cache::getInstance();

        if (is_string($object)) {
            $object = $cache;
        }

        if (!$cache->isEnabled()) {
            return call_user_func($callback, $backtrace->getArguments());
        }

        $hash = md5(
            spl_object_hash($object)
            . $backtrace->getObjectName()
            . $backtrace->getFunctionName()
        );

        if (!$cache->has($object, $hash)) {
            $result = call_user_func($callback, $backtrace->getArguments());

            $cache->set($object, $hash, $result);
        }

        return $cache->get($object, $hash);
    }
}

[solodkiy]

Be aware that spl_object_hash could reuse same id, and do it quite often. I think this makes this function unacceptable for caching.

[henzeb]

As I understand it it's unique as long as that object exists. Once it's gone, it's cache is also gone, thanks to the usage of a WeakMap.

[solodkiy]

Yes, this is correct. And I consider it a big problem, because objects are removing quite frequently.
In my tests, when an object with an ID obtained by spl_object_hash is destructed, the same ID is usually assigned to the next created object of the same class.

[henzeb]

I don't see the problem. once is tied to the object you call it from. If the object is destroyed, so are the cached results. That's how it works. If you want the results to outlive multiple generations of the object, use static methods.

As I said, my code may not generate the hash as thoroughly as the Backtrace does, so it might fail in closure situations, but it works with static methods.