Skip to content
This repository has been archived by the owner on Dec 11, 2020. It is now read-only.

Added batch inserts for doctrine orm populate #1781

Merged
merged 2 commits into from
Sep 2, 2019
Merged

Added batch inserts for doctrine orm populate #1781

merged 2 commits into from
Sep 2, 2019

Conversation

pimjansen
Copy link
Contributor

Added a counter a batch size to insert in batches instead of all at once on which doctrine orm will block itself due the fact it holds all in memory.

Works fine and it reduces the memory consumption a lot. The smaller the batches the less memory it will consume but will be less fast as well.

Only problem that still remains is the return of the PKs inserted though. I don't think there is a real solution for that when you want to keep track of so much data. Did a test with 100k records for this though meaning it holds 100k PKs in an array.

@pimjansen pimjansen self-assigned this Aug 27, 2019
@pimjansen
Copy link
Contributor Author

Will fix #1471

@pimjansen pimjansen added the bug label Aug 27, 2019
Copy link
Contributor

@localheinz localheinz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@fzaninotto
Copy link
Owner

Well, the PK drawback is serious and it should be documented.

@pimjansen
Copy link
Contributor Author

Added some docs of the negative impact on bigger amounts of data

readme.md Outdated Show resolved Hide resolved
src/Faker/ORM/Doctrine/Populator.php Outdated Show resolved Hide resolved
src/Faker/ORM/Doctrine/Populator.php Outdated Show resolved Hide resolved
Copy link
Contributor

@localheinz localheinz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from a sentence which appears to be missing an ending, this looks good to me!

readme.md Outdated Show resolved Hide resolved
src/Faker/ORM/Doctrine/Populator.php Outdated Show resolved Hide resolved
src/Faker/ORM/Doctrine/Populator.php Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
src/Faker/ORM/Doctrine/Populator.php Outdated Show resolved Hide resolved
src/Faker/ORM/Doctrine/Populator.php Outdated Show resolved Hide resolved
@pimjansen
Copy link
Contributor Author

All comments have been handled and applied to the code

@fzaninotto
Copy link
Owner

@pimjansen please don't force push. Just add commits to the PR so that we can see what you did in reaction to the comments. I don't mind having several commits in a PR.

Also, you haven't documented the batchSize anywhere. Considering this is a (slight) breking change, I suggest you mention it in the documentation.

@fzaninotto fzaninotto merged commit ea08301 into fzaninotto:master Sep 2, 2019
@jaapromijn
Copy link

Upgrading from 1.8 to 1.9, this commit seems to have broken our setup:
c49cd54

image

Using the following code:

        $users = $manager->getRepository(User::class)->findAll();

        $populator->addEntity(Status::class, 50, [
            'user' => function () use ($generator, $users) {
                $key = array_rand($users);

                return $users[$key];
            },
        ]);

This addition seems to be the problem:
c49cd54#diff-d38baf8c465e27d0cc5334cf5d8475aaR105

@aless673
Copy link

aless673 commented Jan 16, 2020

Lost 2 hours to understand what was happening. Same here as jaapromijn : 1.9 is now broken for using faker with doctrine relations...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants