Skip to content

#11977 implemented batching of INSERT operations in UnitOfWork#executeInserts() so that EntityPersister#executeInserts() calls are reduced#11978

Merged
greg0ire merged 8 commits intodoctrine:3.5.xfrom
Ocramius:feature/#11977-batch-handling-of-inserts-with-given-ids
Jun 28, 2025
Merged

#11977 implemented batching of INSERT operations in UnitOfWork#executeInserts() so that EntityPersister#executeInserts() calls are reduced#11978
greg0ire merged 8 commits intodoctrine:3.5.xfrom
Ocramius:feature/#11977-batch-handling-of-inserts-with-given-ids

Conversation

@Ocramius
Copy link
Copy Markdown
Member

@Ocramius Ocramius commented Jun 11, 2025

This logic also brings a minor benefit in reducing the number of times ListenersInvoker#getSubscribedSystems is queried.

Implements #11977

TODOs:

  • integration test this - it is expected to reduce the number of EntityPersister#executeInserts() calls
  • refactor this by creating a new @internal class for the batch, and perhaps batch via a generator
  • reduce amount of repeated getClassMetadata() calls
  • reduce overall size of UnitOfWork code, instead of increasing it

The final effect of this patch is that EntityPersister#executeInserts() will be performed on a batch of entities being persisted, which improves efficiency, since a single prepared statement is re-used to perform all INSERT operations.

Ocramius added 3 commits June 11, 2025 15:43
…SERT` operations in `UnitOfWork#executeInserts()`

This logic also brings a minor benefit in reducing the number of times `ListenersInvoker#getSubscribedSystems`
is queried.

TODOs:

* [ ] integration test this - it is expected to reduce the number of `EntityPersister#executeInserts()` calls
* [ ] refactor this by creating a new `@internal` class for the batch, and perhaps batch via a generator
* [ ] reduce amount of repeated `getClassMetadata()` calls
* [ ] reduce overall size of `UnitOfWork` code, instead of increasing it
@Ocramius Ocramius changed the title #11977 implemented simplistic (and ugly) batch handing of INSERT operations in UnitOfWork#executeInserts() #11977 implemented batching of INSERT operations in UnitOfWork#executeInserts() so that EntityPersister#executeInserts() calls are reduced Jun 11, 2025
Ocramius added 2 commits June 11, 2025 18:16
…is in use

The `SequenceGenerator` is potentially used for PostgreSQL table auto-generated fields, but
the `SequenceGenerator` is not a **POST**-insert generator.

Because the `SequenceGenerator` is used in the middle of `INSERT` operations performed
by persisters, we cannot rely on it in batching operations: disabling it, so we get a green
test suite on PostgreSQL.

This change makes `GH10531Test` pass on PostgreSQL: see doctrine#10531
@Ocramius Ocramius marked this pull request as ready for review June 11, 2025 16:35
@Ocramius
Copy link
Copy Markdown
Member Author

I ran a local benchmark, and everything seems fine from a regression PoV:

+-----------------------------------------------------------+-----------------------------------------------------+-----+------+-----+----------+---------------+---------+
| benchmark                                                 | subject                                             | set | revs | its | mem_peak | mode          | rstdev  |
+-----------------------------------------------------------+-----------------------------------------------------+-----+------+-----+----------+---------------+---------+
- | SimpleInsertPerformanceBench                              | benchHydration                                      |     | 5    | 10  | 12.890mb | 25,634.255μs  | ±3.28%  |
+ | SimpleInsertPerformanceBench                              | benchHydration                                      |     | 5    | 10  | 12.890mb | 25,701.962μs  | ±4.43%  |

Copy link
Copy Markdown
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

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

Very nice 👍

@Ocramius
Copy link
Copy Markdown
Member Author

BTW, I'd say that it is a good idea to roll out a minor release with just this change applied, to see if there's a real-world impact already.

Adding batching to the BasicEntityPersister would be next, but I'm unsure of the performance difference between a single $stmt->execute() with a large SQL chunk, vs multiple executions of the same prepared $stmt.

greg0ire
greg0ire previously approved these changes Jun 11, 2025
SenseException
SenseException previously approved these changes Jun 13, 2025
@greg0ire
Copy link
Copy Markdown
Member

BTW, I'd say that it is a good idea to roll out a minor release with just this change applied, to see if there's a real-world impact already.

I just released 3.4.0, so it would be possible to do this by releasing this PR in 3.5.0.

@Ocramius Ocramius changed the base branch from 3.3.x to 3.5.x June 15, 2025 09:16
@Ocramius Ocramius dismissed SenseException’s stale review June 15, 2025 09:16

The base branch was changed.

@Ocramius
Copy link
Copy Markdown
Member Author

Base branch is updated 👍

@Ocramius
Copy link
Copy Markdown
Member Author

@greg0ire can this be milestoned/merged please?

@greg0ire greg0ire merged commit 79e103c into doctrine:3.5.x Jun 28, 2025
84 checks passed
@greg0ire greg0ire added this to the 3.5.0 milestone Jun 28, 2025
@greg0ire
Copy link
Copy Markdown
Member

Sure! Thanks!

@Ocramius Ocramius deleted the feature/#11977-batch-handling-of-inserts-with-given-ids branch June 28, 2025 21:11
@greg0ire
Copy link
Copy Markdown
Member

greg0ire commented Jul 1, 2025

BTW, I'd say that it is a good idea to roll out a minor release with just this change applied, to see if there's a real-world impact already.

Released as https://github.com/doctrine/orm/releases/tag/3.5.0

The change is not alone in this milestone, but I don't think the other changes will impact performance.

@Ocramius
Copy link
Copy Markdown
Member Author

Ocramius commented Jul 1, 2025

Thanks @greg0ire!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants