Skip to content

Commit 79e103c

Browse files
greg0iregreg0ire
authored
Merge pull request #11978 from Ocramius/feature/#11977-batch-handling-of-inserts-with-given-ids
#11977 implemented batching of `INSERT` operations in `UnitOfWork#executeInserts()` so that `EntityPersister#executeInserts()` calls are reduced
2 parents edfaa37 + 79cc70a commit 79e103c

File tree

5 files changed

+274
-14
lines changed

5 files changed

+274
-14
lines changed

‎src/Internal/UnitOfWork/InsertBatch.php‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Doctrine\ORM\Internal\UnitOfWork;
6+
7+
use Doctrine\ORM\EntityManagerInterface;
8+
use Doctrine\ORM\Id\AssignedGenerator;
9+
use Doctrine\ORM\Mapping\ClassMetadata;
10+
11+
/**
12+
* An {@see InsertBatch} represents a set of entities that are safe to be batched
13+
* together in a single query.
14+
*
15+
* These entities are only those that have all fields already assigned, including the
16+
* identifier field(s).
17+
*
18+
* This data structure only exists for internal {@see UnitOfWork} optimisations, and
19+
* should not be relied upon outside the ORM.
20+
*
21+
* @internal
22+
*
23+
* @template TEntity of object
24+
*/
25+
final class InsertBatch
26+
{
27+
/**
28+
* @param ClassMetadata<TEntity> $class
29+
* @param non-empty-list<TEntity> $entities
30+
*/
31+
public function __construct(
32+
public readonly ClassMetadata $class,
33+
public array $entities,
34+
) {
35+
}
36+
37+
/**
38+
* Note: Code in here is procedural/ugly due to it being in a hot path of the {@see UnitOfWork}
39+
*
40+
* This method will batch the given entity set by type, preserving their order. For example,
41+
* given an input [A1, A2, A3, B1, B2, A4, A5], it will create an [[A1, A2, A3], [B1, B2], [A4, A5]] batch.
42+
*
43+
* Entities for which the identifier needs to be generated or fetched by a sequence are put as single
44+
* items in a batch of their own, since it is unsafe to batch-insert them.
45+
*
46+
* @param list<TEntities> $entities
47+
*
48+
* @return list<self<TEntities>>
49+
*
50+
* @template TEntities of object
51+
*/
52+
public static function batchByEntityType(
53+
EntityManagerInterface $entityManager,
54+
array $entities,
55+
): array {
56+
$currentClass = null;
57+
$batches = [];
58+
$batchIndex = -1;
59+
60+
foreach ($entities as $entity) {
61+
$entityClass = $entityManager->getClassMetadata($entity::class);
62+
63+
if (
64+
$currentClass?->name !== $entityClass->name
65+
|| ! $entityClass->idGenerator instanceof AssignedGenerator
66+
) {
67+
$currentClass = $entityClass;
68+
$batches[] = new InsertBatch($entityClass, [$entity]);
69+
$batchIndex += 1;
70+
71+
continue;
72+
}
73+
74+
$batches[$batchIndex]->entities[] = $entity;
75+
}
76+
77+
return $batches;
78+
}
79+
}

‎src/UnitOfWork.php‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
use Doctrine\ORM\Internal\HydrationCompleteHandler;
3232
use Doctrine\ORM\Internal\StronglyConnectedComponents;
3333
use Doctrine\ORM\Internal\TopologicalSort;
34+
use Doctrine\ORM\Internal\UnitOfWork\InsertBatch;
3435
use Doctrine\ORM\Mapping\AssociationMapping;
3536
use Doctrine\ORM\Mapping\ClassMetadata;
3637
use Doctrine\ORM\Mapping\MappingException;
@@ -1037,30 +1038,36 @@ public function recomputeSingleEntityChangeSet(ClassMetadata $class, object $ent
10371038
*/
10381039
private function executeInserts(): void
10391040
{
1040-
$entities = $this->computeInsertExecutionOrder();
1041+
$batchedByType = InsertBatch::batchByEntityType($this->em, $this->computeInsertExecutionOrder());
10411042
$eventsToDispatch = [];
10421043

1043-
foreach ($entities as $entity) {
1044-
$oid = spl_object_id($entity);
1045-
$class = $this->em->getClassMetadata($entity::class);
1044+
foreach ($batchedByType as $batch) {
1045+
$class = $batch->class;
1046+
$invoke = $this->listenersInvoker->getSubscribedSystems($class, Events::postPersist);
10461047
$persister = $this->getEntityPersister($class->name);
10471048

1048-
$persister->addInsert($entity);
1049+
foreach ($batch->entities as $entity) {
1050+
$oid = spl_object_id($entity);
10491051

1050-
unset($this->entityInsertions[$oid]);
1052+
$persister->addInsert($entity);
1053+
1054+
unset($this->entityInsertions[$oid]);
1055+
}
10511056

10521057
$persister->executeInserts();
10531058

1054-
if (! isset($this->entityIdentifiers[$oid])) {
1055-
//entity was not added to identity map because some identifiers are foreign keys to new entities.
1056-
//add it now
1057-
$this->addToEntityIdentifiersAndEntityMap($class, $oid, $entity);
1058-
}
1059+
foreach ($batch->entities as $entity) {
1060+
$oid = spl_object_id($entity);
10591061

1060-
$invoke = $this->listenersInvoker->getSubscribedSystems($class, Events::postPersist);
1062+
if (! isset($this->entityIdentifiers[$oid])) {
1063+
//entity was not added to identity map because some identifiers are foreign keys to new entities.
1064+
//add it now
1065+
$this->addToEntityIdentifiersAndEntityMap($class, $oid, $entity);
1066+
}
10611067

1062-
if ($invoke !== ListenersInvoker::INVOKE_NONE) {
1063-
$eventsToDispatch[] = ['class' => $class, 'entity' => $entity, 'invoke' => $invoke];
1068+
if ($invoke !== ListenersInvoker::INVOKE_NONE) {
1069+
$eventsToDispatch[] = ['class' => $class, 'entity' => $entity, 'invoke' => $invoke];
1070+
}
10641071
}
10651072
}
10661073

‎tests/Tests/Mocks/EntityPersisterMock.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
*/
1414
class EntityPersisterMock extends BasicEntityPersister
1515
{
16+
/** @var int<0, max> */
17+
private int $countOfExecuteInsertCalls = 0;
1618
private array $inserts = [];
1719
private array $updates = [];
1820
private array $deletes = [];
@@ -40,6 +42,8 @@ public function addInsert(object $entity): void
4042

4143
public function executeInserts(): void
4244
{
45+
$this->countOfExecuteInsertCalls += 1;
46+
4347
foreach ($this->postInsertIds as $item) {
4448
$this->em->getUnitOfWork()->assignPostInsertId($item['entity'], $item['generatedId']);
4549
}
@@ -86,6 +90,7 @@ public function getDeletes(): array
8690

8791
public function reset(): void
8892
{
93+
$this->countOfExecuteInsertCalls = 0;
8994
$this->existsCalled = false;
9095
$this->identityColumnValueCounter = 0;
9196
$this->inserts = [];
@@ -97,4 +102,10 @@ public function isExistsCalled(): bool
97102
{
98103
return $this->existsCalled;
99104
}
105+
106+
/** @return int<0, max> */
107+
public function countOfExecuteInsertCalls(): int
108+
{
109+
return $this->countOfExecuteInsertCalls;
110+
}
100111
}

‎tests/Tests/ORM/Internal/UnitOfWork/InsertBatchTest.php‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Doctrine\Tests\ORM\Internal\UnitOfWork;
6+
7+
use Doctrine\ORM\EntityManagerInterface;
8+
use Doctrine\ORM\Id\AssignedGenerator;
9+
use Doctrine\ORM\Id\IdentityGenerator;
10+
use Doctrine\ORM\Internal\UnitOfWork\InsertBatch;
11+
use Doctrine\ORM\Mapping\ClassMetadata;
12+
use PHPUnit\Framework\Attributes\CoversClass;
13+
use PHPUnit\Framework\Attributes\Group;
14+
use PHPUnit\Framework\MockObject\Stub;
15+
use PHPUnit\Framework\TestCase;
16+
17+
#[CoversClass(InsertBatch::class)]
18+
#[Group('#11977')]
19+
final class InsertBatchTest extends TestCase
20+
{
21+
private EntityManagerInterface&Stub $entityManager;
22+
23+
protected function setUp(): void
24+
{
25+
$this->entityManager = $this->createStub(EntityManagerInterface::class);
26+
27+
$entityAMetadata = new ClassMetadata(EntityA::class);
28+
$entityBMetadata = new ClassMetadata(EntityB::class);
29+
$entityCMetadata = new ClassMetadata(EntityC::class);
30+
31+
$entityAMetadata->idGenerator = new AssignedGenerator();
32+
$entityBMetadata->idGenerator = new AssignedGenerator();
33+
$entityCMetadata->idGenerator = new IdentityGenerator();
34+
35+
$this->entityManager->method('getClassMetadata')
36+
->willReturnMap([
37+
[EntityA::class, $entityAMetadata],
38+
[EntityB::class, $entityBMetadata],
39+
[EntityC::class, $entityCMetadata],
40+
]);
41+
}
42+
43+
public function testWillProduceEmptyBatchOnNoGivenEntities(): void
44+
{
45+
self::assertEmpty(InsertBatch::batchByEntityType($this->entityManager, []));
46+
}
47+
48+
public function testWillBatchSameEntityOperationsInSingleBatch(): void
49+
{
50+
$batches = InsertBatch::batchByEntityType(
51+
$this->entityManager,
52+
[
53+
new EntityA(),
54+
new EntityA(),
55+
new EntityA(),
56+
],
57+
);
58+
59+
self::assertCount(1, $batches);
60+
self::assertSame(EntityA::class, $batches[0]->class->name);
61+
self::assertCount(3, $batches[0]->entities);
62+
}
63+
64+
public function testWillBatchInterleavedEntityOperationsInGroups(): void
65+
{
66+
$batches = InsertBatch::batchByEntityType(
67+
$this->entityManager,
68+
[
69+
new EntityA(),
70+
new EntityA(),
71+
new EntityB(),
72+
new EntityB(),
73+
new EntityA(),
74+
new EntityA(),
75+
],
76+
);
77+
78+
self::assertCount(3, $batches);
79+
self::assertSame(EntityA::class, $batches[0]->class->name);
80+
self::assertCount(2, $batches[0]->entities);
81+
self::assertSame(EntityB::class, $batches[1]->class->name);
82+
self::assertCount(2, $batches[1]->entities);
83+
self::assertSame(EntityA::class, $batches[2]->class->name);
84+
self::assertCount(2, $batches[2]->entities);
85+
}
86+
87+
public function testWillNotBatchOperationsForAGeneratedIdentifierEntity(): void
88+
{
89+
$batches = InsertBatch::batchByEntityType(
90+
$this->entityManager,
91+
[
92+
new EntityC(),
93+
new EntityC(),
94+
new EntityC(),
95+
],
96+
);
97+
98+
self::assertCount(3, $batches);
99+
self::assertSame(EntityC::class, $batches[0]->class->name);
100+
self::assertCount(1, $batches[0]->entities);
101+
self::assertSame(EntityC::class, $batches[1]->class->name);
102+
self::assertCount(1, $batches[1]->entities);
103+
self::assertSame(EntityC::class, $batches[2]->class->name);
104+
self::assertCount(1, $batches[2]->entities);
105+
}
106+
107+
public function testWillIsolateBatchesForEntitiesWithGeneratedIdentifiers(): void
108+
{
109+
$batches = InsertBatch::batchByEntityType(
110+
$this->entityManager,
111+
[
112+
new EntityA(),
113+
new EntityA(),
114+
new EntityC(),
115+
new EntityC(),
116+
new EntityA(),
117+
new EntityA(),
118+
],
119+
);
120+
121+
self::assertCount(4, $batches);
122+
self::assertSame(EntityA::class, $batches[0]->class->name);
123+
self::assertCount(2, $batches[0]->entities);
124+
self::assertSame(EntityC::class, $batches[1]->class->name);
125+
self::assertCount(1, $batches[1]->entities);
126+
self::assertSame(EntityC::class, $batches[2]->class->name);
127+
self::assertCount(1, $batches[2]->entities);
128+
self::assertSame(EntityA::class, $batches[3]->class->name);
129+
self::assertCount(2, $batches[3]->entities);
130+
}
131+
}
132+
133+
class EntityA
134+
{
135+
}
136+
137+
class EntityB
138+
{
139+
}
140+
141+
class EntityC
142+
{
143+
}

‎tests/Tests/ORM/UnitOfWorkTest.php‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
use Doctrine\Tests\Models\CMS\CmsUser;
3434
use Doctrine\Tests\Models\Forum\ForumAvatar;
3535
use Doctrine\Tests\Models\Forum\ForumUser;
36+
use Doctrine\Tests\Models\MixedToOneIdentity\Country;
3637
use Doctrine\Tests\OrmTestCase;
3738
use Exception as BaseException;
3839
use PHPUnit\Framework\Attributes\DataProvider;
@@ -143,6 +144,25 @@ public function testSavingSingleEntityWithIdentityColumnForcesInsert(): void
143144
self::assertIsNumeric($user->id);
144145
}
145146

147+
#[Group('#11977')]
148+
public function testMultipleInsertsAreBatchedInThePersister(): void
149+
{
150+
$userPersister = new EntityPersisterMock($this->_emMock, $this->_emMock->getClassMetadata(Country::class));
151+
$this->_unitOfWork->setEntityPersister(Country::class, $userPersister);
152+
153+
$country1 = new Country();
154+
$country1->country = 'Italy';
155+
$country2 = new Country();
156+
$country2->country = 'Germany';
157+
158+
$this->_unitOfWork->persist($country1);
159+
$this->_unitOfWork->persist($country2);
160+
$this->_unitOfWork->commit();
161+
162+
self::assertCount(2, $userPersister->getInserts());
163+
self::assertSame(1, $userPersister->countOfExecuteInsertCalls());
164+
}
165+
146166
/**
147167
* Tests a scenario where a save() operation is cascaded from a ForumUser
148168
* to its associated ForumAvatar, both entities using IDENTITY id generation.

0 commit comments

Comments
 (0)