Performance: Reduce number of database calls in save and publish operations #20485
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR improves database performance in Umbraco CMS save and publish operations by reducing the number of SQL queries through request caching and batch operations.
- Added request caching for media lookups in MediaPicker3PropertyEditor to avoid redundant database calls when the same media is referenced multiple times
- Converted individual property data insert/update operations to bulk statements, reducing from potentially hundreds of separate SQL calls to just two batch operations
- Added integration tests to verify the new batch update functionality works correctly
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| MediaPicker3PropertyEditor.cs | Implements request caching for media lookups to prevent duplicate database queries |
| ContentRepositoryBase.cs | Replaces individual insert/update operations with bulk database operations for property data |
| NPocoUpdateBatchTests.cs | New integration test to verify batch update functionality |
| NPocoFetchTests.cs | Removed unused imports |
...aco.Tests.Integration/Umbraco.Infrastructure/Persistence/NPocoTests/NPocoUpdateBatchTests.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs
Show resolved
Hide resolved
Zeegaan
approved these changes
Oct 14, 2025
Zeegaan
pushed a commit
that referenced
this pull request
Oct 14, 2025
…ations (#20485) * Added request caching to media picker media retrieval, to improve performance in save operations. * WIP: Update or insert in bulk when updating property data. * Add tests verifying UpdateBatch. * Fixed issue with UpdateBatch and SQL Server. * Removed stopwatch. * Fix test on SQLite (failing on SQLServer). * Added temporary test for direct call to NPoco UpdateBatch. * Fixed test on SQLServer. * Add integration test verifying the same property data is persisted as before the performance refactor. * Log expected warning in DocumentUrlService as debug. (cherry picked from commit 12adfd5)
This was referenced Oct 16, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Following up #20405, this PR is looking to add further performance updates by reducing the number of SQL calls we make in save and publish operations. In investigating a support case with a lot of blocks and languages, it's apparent that with latency between database and web server, this can cause a performance impact.
Firstly I've added further request caching on the media picker - with the artificially slow setup using a local installation and a database in Azure, this reduces the time to about a third (20-21s to 6-7s).
Secondly I'm looking to convert property data updates to bulk statements, so there's one for all inserts and one for updates, rather than 110 separate ones (with this project setup). This is less impressive but shaves off a few seconds for this part of the operation (9s to 7s). I've added an integration test for that that checks the raw data records in the property data table to verify this change hasn't affected behaviour.
I've reviewed but either haven't seen scope for improvement or haven't seen as significant the events that happen via notification handlers: