Skip to content

Commit 4a24860

Browse files
OcramiusOcramius
committed
#11977 isolated INSERT batch generation to own @internal performance-sensitive class
1 parent 116cdf8 commit 4a24860

File tree

3 files changed

+187
-29
lines changed

3 files changed

+187
-29
lines changed

‎src/Internal/UnitOfWork/InsertBatch.php‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
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\Mapping\ClassMetadata;
9+
10+
/**
11+
* An {@see InsertBatch} represents a set of entities that are safe to be batched
12+
* together in a single query.
13+
*
14+
* These entities are only those that have all fields already assigned, including the
15+
* identifier field(s).
16+
*
17+
* This data structure only exists for internal {@see UnitOfWork} optimisations, and
18+
* should not be relied upon outside the ORM.
19+
*
20+
* @internal
21+
*
22+
* @template TEntity of object
23+
*/
24+
final class InsertBatch
25+
{
26+
/**
27+
* @param ClassMetadata<TEntity> $class
28+
* @param non-empty-list<TEntity> $entities
29+
*/
30+
public function __construct(
31+
public readonly ClassMetadata $class,
32+
public array $entities,
33+
) {
34+
}
35+
36+
/**
37+
* Note: Code in here is procedural/ugly due to it being in a hot path of the {@see UnitOfWork}
38+
*
39+
* @param list<TEntities> $entities
40+
*
41+
* @return list<self<TEntities>>
42+
*
43+
* @template TEntities of object
44+
*/
45+
public static function batchByEntityType(
46+
EntityManagerInterface $entityManager,
47+
array $entities,
48+
): array {
49+
$currentClass = null;
50+
$batches = [];
51+
$batchIndex = -1;
52+
53+
foreach ($entities as $entity) {
54+
$entityClass = $entityManager->getClassMetadata($entity::class);
55+
56+
if (
57+
$currentClass?->name !== $entityClass->name
58+
|| $entityClass->idGenerator->isPostInsertGenerator()
59+
) {
60+
$currentClass = $entityClass;
61+
$batches[] = new InsertBatch($entityClass, [$entity]);
62+
$batchIndex += 1;
63+
64+
continue;
65+
}
66+
67+
$batches[$batchIndex]->entities[] = $entity;
68+
}
69+
70+
return $batches;
71+
}
72+
}

‎src/UnitOfWork.php‎

Lines changed: 6 additions & 29 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;
@@ -61,7 +62,6 @@
6162
use function array_sum;
6263
use function array_values;
6364
use function assert;
64-
use function count;
6565
use function current;
6666
use function get_debug_type;
6767
use function implode;
@@ -1038,41 +1038,18 @@ public function recomputeSingleEntityChangeSet(ClassMetadata $class, object $ent
10381038
*/
10391039
private function executeInserts(): void
10401040
{
1041-
$entities = $this->computeInsertExecutionOrder();
10421041
$eventsToDispatch = [];
1043-
// @TODO #11977 this is ugly and to be tested: making it work, then refactoring later.
1044-
// We assemble micro-batches to process together: this should probably occur in a `private` function?
10451042
// @TODO #11977 test this by verifying the number of executed SQL queries in a flush operation?
10461043
// @TODO #11977 could this be applied also to batch updates? If so, let's raise a new issue.
1047-
/** @var list<array{class: Mapping\ClassMetadata, entities: non-empty-list<object>}> $batchedByType */
1048-
$batchedByType = [];
1049-
1050-
foreach ($entities as $entity) {
1051-
$currentClass = ($batchedByType[count($batchedByType) - 1]['class'] ?? null)?->getName();
1052-
$entityClass = $this->em->getClassMetadata($entity::class);
1053-
1054-
if (
1055-
$currentClass !== $entityClass->name
1056-
// don't batch things together, if an ID generator is needed
1057-
|| $entityClass->idGenerator->isPostInsertGenerator()
1058-
) {
1059-
$batchedByType[] = [
1060-
'class' => $entityClass,
1061-
'entities' => [$entity],
1062-
];
1063-
1064-
continue;
1065-
}
1066-
1067-
$batchedByType[count($batchedByType) - 1]['entities'][] = $entity;
1068-
}
1044+
/** @var list<InsertBatch<object>> $batchedByType */
1045+
$batchedByType = InsertBatch::batchByEntityType($this->em, $this->computeInsertExecutionOrder());
10691046

10701047
foreach ($batchedByType as $batch) {
1071-
$class = $batch['class'];
1048+
$class = $batch->class;
10721049
$invoke = $this->listenersInvoker->getSubscribedSystems($class, Events::postPersist);
10731050
$persister = $this->getEntityPersister($class->name);
10741051

1075-
foreach ($batch['entities'] as $entity) {
1052+
foreach ($batch->entities as $entity) {
10761053
$oid = spl_object_id($entity);
10771054

10781055
$persister->addInsert($entity);
@@ -1082,7 +1059,7 @@ private function executeInserts(): void
10821059

10831060
$persister->executeInserts();
10841061

1085-
foreach ($batch['entities'] as $entity) {
1062+
foreach ($batch->entities as $entity) {
10861063
$oid = spl_object_id($entity);
10871064

10881065
if (! isset($this->entityIdentifiers[$oid])) {

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

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
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\MockObject\Stub;
14+
use PHPUnit\Framework\TestCase;
15+
16+
#[CoversClass(InsertBatch::class)]
17+
final class InsertBatchTest extends TestCase
18+
{
19+
private EntityManagerInterface&Stub $entityManager;
20+
21+
protected function setUp(): void
22+
{
23+
$this->entityManager = $this->createStub(EntityManagerInterface::class);
24+
25+
$entityAMetadata = new ClassMetadata(EntityA::class);
26+
$entityBMetadata = new ClassMetadata(EntityB::class);
27+
$entityCMetadata = new ClassMetadata(EntityC::class);
28+
29+
$entityAMetadata->idGenerator = new AssignedGenerator();
30+
$entityBMetadata->idGenerator = new AssignedGenerator();
31+
$entityCMetadata->idGenerator = new IdentityGenerator();
32+
33+
$this->entityManager->method('getClassMetadata')
34+
->willReturnMap([
35+
[EntityA::class, $entityAMetadata],
36+
[EntityB::class, $entityBMetadata],
37+
[EntityC::class, $entityCMetadata],
38+
]);
39+
}
40+
41+
public function testWillProduceEmptyBatchOnNoGivenEntities(): void
42+
{
43+
self::assertEmpty(InsertBatch::batchByEntityType($this->entityManager, []));
44+
}
45+
46+
public function testWillBatchSameEntityOperationsInSingleBatch(): void
47+
{
48+
$batches = InsertBatch::batchByEntityType(
49+
$this->entityManager,
50+
[
51+
new EntityA(),
52+
new EntityA(),
53+
new EntityA(),
54+
],
55+
);
56+
57+
self::assertCount(1, $batches);
58+
self::assertCount(3, $batches[0]->entities);
59+
}
60+
61+
public function testWillBatchInterleavedEntityOperationsInGroups(): void
62+
{
63+
$batches = InsertBatch::batchByEntityType(
64+
$this->entityManager,
65+
[
66+
new EntityA(),
67+
new EntityA(),
68+
new EntityB(),
69+
new EntityB(),
70+
new EntityA(),
71+
new EntityA(),
72+
],
73+
);
74+
75+
self::assertCount(3, $batches);
76+
self::assertCount(2, $batches[0]->entities);
77+
self::assertCount(2, $batches[1]->entities);
78+
self::assertCount(2, $batches[2]->entities);
79+
}
80+
81+
public function testWillNotBatchOperationsForAGeneratedIdentifierEntity(): void
82+
{
83+
$batches = InsertBatch::batchByEntityType(
84+
$this->entityManager,
85+
[
86+
new EntityC(),
87+
new EntityC(),
88+
new EntityC(),
89+
],
90+
);
91+
92+
self::assertCount(3, $batches);
93+
self::assertCount(1, $batches[0]->entities);
94+
self::assertCount(1, $batches[1]->entities);
95+
self::assertCount(1, $batches[2]->entities);
96+
}
97+
}
98+
99+
class EntityA
100+
{
101+
}
102+
103+
class EntityB
104+
{
105+
}
106+
107+
class EntityC
108+
{
109+
}

0 commit comments

Comments
 (0)