yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Performance: Reduce number of database calls in save and publish operations (#20485)

Browse Source 
* 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 12adfd52bd)
This commit is contained in:
Andy Butland
2025-10-14 11:22:21 +02:00
committed by Zeegaan
parent ea44850804
commit 37b239b8ca
6 changed files with 142 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
src/Umbraco.Core/Services/DocumentUrlService.cs
View File

@@ -432,7 +432,8 @@ public class DocumentUrlService : IDocumentUrlService
if (draftUrlSegments.Any() is false)
{
_logger.LogWarning("No draft URL segments found for document {DocumentKey} in culture {Culture}", document.Key, culture ?? "{null}");
// Log at debug level because this is expected when a document is not published in a given language.
_logger.LogDebug("No draft URL segments found for document {DocumentKey} in culture {Culture}", document.Key, culture ?? "{null}");
}
else
{

21
src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs
View File

@@ -1155,25 +1155,38 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement
IEnumerable<PropertyDataDto> propertyDataDtos = PropertyFactory.BuildDtos(entity.ContentType.Variations, entity.VersionId, publishedVersionId, entity.Properties, LanguageRepository, out edited, out editedCultures);
var toUpdate = new List<PropertyDataDto>();
var toInsert = new List<PropertyDataDto>();
foreach (PropertyDataDto propertyDataDto in propertyDataDtos)
{
// Check if this already exists and update, else insert a new one
if (propertyTypeToPropertyData.TryGetValue((propertyDataDto.PropertyTypeId, propertyDataDto.VersionId, propertyDataDto.LanguageId, propertyDataDto.Segment), out PropertyDataDto? propData))
{
propertyDataDto.Id = propData.Id;
Database.Update(propertyDataDto);
toUpdate.Add(propertyDataDto);
}
else
{
// TODO: we can speed this up: Use BulkInsert and then do one SELECT to re-retrieve the property data inserted with assigned IDs.
// This is a perfect thing to benchmark with Benchmark.NET to compare perf between Nuget releases.
Database.Insert(propertyDataDto);
toInsert.Add(propertyDataDto);
}
// track which ones have been processed
existingPropDataIds.Remove(propertyDataDto.Id);
}
if (toUpdate.Count > 0)
{
var updateBatch = toUpdate
.Select(x => UpdateBatch.For(x))
.ToList();
Database.UpdateBatch(updateBatch, new BatchOptions { BatchSize = 100 });
}
if (toInsert.Count > 0)
{
Database.InsertBulk(toInsert);
}
// For any remaining that haven't been processed they need to be deleted
if (existingPropDataIds.Count > 0)
{

27
src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs
View File

@@ -54,6 +54,8 @@ public class MediaPicker3PropertyEditor : DataEditor
/// </summary>
internal sealed class MediaPicker3PropertyValueEditor : DataValueEditor, IDataValueReference
{
private const string MediaCacheKeyFormat = nameof(MediaPicker3PropertyValueEditor) + "_Media_{0}";
private readonly IDataTypeConfigurationCache _dataTypeReadCache;
private readonly IJsonSerializer _jsonSerializer;
private readonly IMediaImportService _mediaImportService;
@@ -61,6 +63,7 @@ public class MediaPicker3PropertyEditor : DataEditor
private readonly ITemporaryFileService _temporaryFileService;
private readonly IScopeProvider _scopeProvider;
private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor;
private readonly AppCaches _appCaches;
/// <summary>
/// Initializes a new instance of the <see cref="MediaPicker3PropertyValueEditor"/> class.
@@ -93,6 +96,8 @@ public class MediaPicker3PropertyEditor : DataEditor
_scopeProvider = scopeProvider;
_backOfficeSecurityAccessor = backOfficeSecurityAccessor;
_dataTypeReadCache = dataTypeReadCache;
_appCaches = appCaches;
var validators = new TypedJsonValidatorRunner<List<MediaWithCropsDto>, MediaPicker3Configuration>(
jsonSerializer,
new MinMaxValidator(localizedTextService),
@@ -203,13 +208,31 @@ public class MediaPicker3PropertyEditor : DataEditor
foreach (MediaWithCropsDto mediaWithCropsDto in mediaWithCropsDtos)
{
IMedia? media = _mediaService.GetById(mediaWithCropsDto.MediaKey);
IMedia? media = GetMediaById(mediaWithCropsDto.MediaKey);
mediaWithCropsDto.MediaTypeAlias = media?.ContentType.Alias ?? unknownMediaType;
}
return mediaWithCropsDtos.Where(m => m.MediaTypeAlias != unknownMediaType).ToList();
}
private IMedia? GetMediaById(Guid key)
{
// Cache media lookups in case the same media is handled multiple times across a save operation,
// which is possible, particularly if we have multiple languages and blocks.
var cacheKey = string.Format(MediaCacheKeyFormat, key);
IMedia? media = _appCaches.RequestCache.GetCacheItem<IMedia?>(cacheKey);
if (media is null)
{
media = _mediaService.GetById(key);
if (media is not null)
{
_appCaches.RequestCache.Set(cacheKey, media);
}
}
return media;
}
private List<MediaWithCropsDto> HandleTemporaryMediaUploads(List<MediaWithCropsDto> mediaWithCropsDtos, MediaPicker3Configuration configuration)
{
var invalidDtos = new List<MediaWithCropsDto>();
@@ -217,7 +240,7 @@ public class MediaPicker3PropertyEditor : DataEditor
foreach (MediaWithCropsDto mediaWithCropsDto in mediaWithCropsDtos)
{
// if the media already exist, don't bother with it
if (_mediaService.GetById(mediaWithCropsDto.MediaKey) != null)
if (GetMediaById(mediaWithCropsDto.MediaKey) != null)
{
continue;
}

28
tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentPublishingServiceTests.Publish.cs
View File

@@ -3,6 +3,7 @@ using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Models.ContentPublishing;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Services.OperationStatus;
using Umbraco.Cms.Infrastructure.Persistence.Dtos;
using Umbraco.Cms.Tests.Integration.Testing;
namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.Services;
@@ -388,4 +389,31 @@ public partial class ContentPublishingServiceTests : UmbracoIntegrationTestWithC
?? throw new InvalidOperationException("Expected a publish date for EN");
Assert.Greater(lastPublishDateEn, firstPublishDateEn);
}
[Test]
public async Task Publishing_Single_Culture_Persists_Expected_Property_Data()
{
var (langEn, langDa, langBe, contentType) = await SetupVariantDoctypeAsync();
var content = await CreateVariantContentAsync(langEn, langDa, langBe, contentType);
using (var scope = ScopeProvider.CreateScope())
{
var dtos = scope.Database.Fetch<PropertyDataDto>();
Assert.AreEqual(18, dtos.Count); // 3 properties * 3 cultures * 2 (published + edited).
scope.Complete();
}
var publishAttempt = await ContentPublishingService.PublishAsync(
content.Key,
[new() { Culture = langEn.IsoCode }],
Constants.Security.SuperUserKey);
Assert.IsTrue(publishAttempt.Success);
using (var scope = ScopeProvider.CreateScope())
{
var dtos = scope.Database.Fetch<PropertyDataDto>();
Assert.AreEqual(19, dtos.Count); // + 3 for published populated title property, - 2 for existing published properties of other cultures.
scope.Complete();
}
}
}

3
tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/NPocoTests/NPocoFetchTests.cs
View File

@@ -1,13 +1,10 @@
// Copyright (c) Umbraco.
// See LICENSE for more details.
using System.Collections.Generic;
using System.Linq;
using NPoco;
using NUnit.Framework;
using Umbraco.Cms.Tests.Common.Testing;
using Umbraco.Cms.Tests.Integration.Testing;
using Umbraco.Extensions;
namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Persistence.NPocoTests;

70
tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/NPocoTests/NPocoUpdateBatchTests.cs Normal file
View File

@@ -0,0 +1,70 @@
// Copyright (c) Umbraco.
// See LICENSE for more details.
using NPoco;
using NUnit.Framework;
using Umbraco.Cms.Infrastructure.Persistence.Dtos;
using Umbraco.Cms.Tests.Common.Testing;
using Umbraco.Cms.Tests.Integration.Testing;
namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Persistence.NPocoTests;
[TestFixture]
[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)]
internal sealed class NPocoUpdateBatchTests : UmbracoIntegrationTest
{
[Test]
public void Can_Update_Batch()
{
// Arrange
var servers = new List<ServerRegistrationDto>();
for (var i = 0; i < 3; i++)
{
servers.Add(new ServerRegistrationDto
{
Id = i + 1,
ServerAddress = "address" + i,
ServerIdentity = "computer" + i,
DateRegistered = DateTime.Now,
IsActive = false,
DateAccessed = DateTime.Now,
});
}
using (var scope = ScopeProvider.CreateScope())
{
scope.Database.BulkInsertRecords(servers);
scope.Complete();
}
// Act
for (var i = 0; i < 3; i++)
{
servers[i].ServerAddress = "newaddress" + i;
servers[i].IsActive = true;
}
using (var scope = ScopeProvider.CreateScope())
{
var updateBatch = servers
.Select(x => UpdateBatch.For(x))
.ToList();
var updated = scope.Database.UpdateBatch(updateBatch, new BatchOptions { BatchSize = 100 });
Assert.AreEqual(3, updated);
scope.Complete();
}
// Assert
using (var scope = ScopeProvider.CreateScope())
{
var dtos = scope.Database.Fetch<ServerRegistrationDto>();
Assert.AreEqual(3, dtos.Count);
for (var i = 0; i < 3; i++)
{
Assert.AreEqual(servers[i].ServerAddress, dtos[i].ServerAddress);
Assert.AreEqual(servers[i].ServerIdentity, dtos[i].ServerIdentity);
Assert.AreEqual(servers[i].IsActive, dtos[i].IsActive);
}
}
}
}