zenstruck / foundry

A model factory library for creating expressive, auto-completable, on-demand dev/test fixtures with Symfony and Doctrine.
https://symfony.com/bundles/ZenstruckFoundryBundle/current/index.html
MIT License
607 stars 62 forks source link

Performance Problem with random #612

Open amyAMeQ opened 2 weeks ago

amyAMeQ commented 2 weeks ago

While diagnosing why some of our tests are so slow, I discovered the culprit for us is with random(). We have a function:

public static function randomApiUser(): ApiUser
    {
        $user = self::random()->object();
        return new ApiUser($user);
    }

And it was making tests take five times as long as when we didn't use it.

After poking around, I noticed that rather than getting X random records, it's getting ALL of them and then getting random ones from that.

public function randomRange(int $min, int $max, array $attributes = []): array
    {
        if ($min < 0) {
            throw new \InvalidArgumentException(\sprintf('$min must be positive (%d given).', $min));
        }

        if ($max < $min) {
            throw new \InvalidArgumentException(\sprintf('$max (%d) cannot be less than $min (%d).', $max, $min));
        }

        $all = \array_values($this->findBy($attributes));

        \shuffle($all);

        if (\count($all) < $max) {
            throw new \RuntimeException(\sprintf('At least %d "%s" object(s) must have been persisted (%d persisted).', $max, $this->getClassName(), \count($all)));
        }

        return \array_slice($all, 0, \random_int($min, $max)); // @phpstan-ignore-line
    }

I think it would be more performant if the code used something like this to just get random records from the database (with the appropriate attribute filters):

SELECT * 
FROM table_name
ORDER BY RAND()
LIMIT 1;

I am willing to do a PR for this if you would like.

nikophil commented 2 weeks ago

Hi @amyAMeQ

thanks for the report, you're totally right about this, it could be improved.

Problem is: I think it is not the same syntax between mysql (ORDER BY RAND()) and postgresql (ORDER BY RANDOM())

We're about to release Foundry 2.0, certainly the next week. I'd rather this to be fixed in 2.0 than in 1.x

amyAMeQ commented 2 weeks ago

@nikophil - OK. I will pull the 2.0 branch and play locally. Hoping I can find a way to do it in a database agnostic way.

nikophil commented 2 weeks ago

you'll find an upgrade guide here: https://github.com/zenstruck/foundry/blob/1.x/UPGRADE-2.0.md (not 100% up to date)

BTW don't hesitate to report any problems you'd find in 2.x branch :sweat_smile: