A bunch of minor performance optimizations (#16335)

* Do not execute query if no macros found

* Request cache the permission lookup

* Unbreak change by adding obsolete ctor

* Clean up

* Wrap indexing for delivery API in a scope

* Do not ask options every time for the timeout, instead listen for updates

* Lookup content types once instead of one by one

* Use TryGetValue instead

* Do a distinct on user ids before building index, to avoid issue with more than 2100 parameters

* Don't map ContentDto (it's unused)

* Introduce request bound block editor element cache

---------

Co-authored-by: kjac <kja@umbraco.dk>

src/Umbraco.Cms.Persistence.EFCore/Locking/SqlServerEFCoreDistributedLockingMechanism.cs

@@ -16,8 +16,8 @@ namespace Umbraco.Cms.Persistence.EFCore.Locking;
 internal class SqlServerEFCoreDistributedLockingMechanism<T> : IDistributedLockingMechanism
     where T : DbContext
 {
-    private readonly IOptionsMonitor<ConnectionStrings> _connectionStrings;
-    private readonly IOptionsMonitor<GlobalSettings> _globalSettings;
+    private ConnectionStrings _connectionStrings;
+    private GlobalSettings _globalSettings;
     private readonly ILogger<SqlServerEFCoreDistributedLockingMechanism<T>> _logger;
     private readonly Lazy<IEFCoreScopeAccessor<T>> _scopeAccessor; // Hooray it's a circular dependency.
 
@@ -32,27 +32,29 @@ internal class SqlServerEFCoreDistributedLockingMechanism<T> : IDistributedLocki
     {
         _logger = logger;
         _scopeAccessor = scopeAccessor;
-        _globalSettings = globalSettings;
-        _connectionStrings = connectionStrings;
+        _globalSettings = globalSettings.CurrentValue;
+        _connectionStrings = connectionStrings.CurrentValue;
+        globalSettings.OnChange(x=>_globalSettings = x);
+        connectionStrings.OnChange(x=>_connectionStrings = x);
     }
 
     public bool HasActiveRelatedScope => _scopeAccessor.Value.AmbientScope is not null;
 
     /// <inheritdoc />
-    public bool Enabled => _connectionStrings.CurrentValue.IsConnectionStringConfigured() &&
-                           string.Equals(_connectionStrings.CurrentValue.ProviderName, "Microsoft.Data.SqlClient", StringComparison.InvariantCultureIgnoreCase) && _scopeAccessor.Value.AmbientScope is not null;
+    public bool Enabled => _connectionStrings.IsConnectionStringConfigured() &&
+                           string.Equals(_connectionStrings.ProviderName, "Microsoft.Data.SqlClient", StringComparison.InvariantCultureIgnoreCase) && _scopeAccessor.Value.AmbientScope is not null;
 
     /// <inheritdoc />
     public IDistributedLock ReadLock(int lockId, TimeSpan? obtainLockTimeout = null)
     {
-        obtainLockTimeout ??= _globalSettings.CurrentValue.DistributedLockingReadLockDefaultTimeout;
+        obtainLockTimeout ??= _globalSettings.DistributedLockingReadLockDefaultTimeout;
         return new SqlServerDistributedLock(this, lockId, DistributedLockType.ReadLock, obtainLockTimeout.Value);
     }
 
     /// <inheritdoc />
     public IDistributedLock WriteLock(int lockId, TimeSpan? obtainLockTimeout = null)
     {
-        obtainLockTimeout ??= _globalSettings.CurrentValue.DistributedLockingWriteLockDefaultTimeout;
+        obtainLockTimeout ??= _globalSettings.DistributedLockingWriteLockDefaultTimeout;
         return new SqlServerDistributedLock(this, lockId, DistributedLockType.WriteLock, obtainLockTimeout.Value);
     }
 
@@ -168,9 +170,7 @@ private void ObtainWriteLock()
                         "A transaction with minimum ReadCommitted isolation level is required.");
                 }
 
-                await dbContext.Database.ExecuteSqlRawAsync($"SET LOCK_TIMEOUT {(int)_timeout.TotalMilliseconds};");
-
-                var rowsAffected = await dbContext.Database.ExecuteSqlAsync(@$"UPDATE umbracoLock WITH (REPEATABLEREAD) SET value = (CASE WHEN (value=1) THEN -1 ELSE 1 END) WHERE id={LockId}");
+                var rowsAffected = await dbContext.Database.ExecuteSqlAsync(@$"SET LOCK_TIMEOUT {(int)_timeout.TotalMilliseconds};UPDATE umbracoLock WITH (REPEATABLEREAD) SET value = (CASE WHEN (value=1) THEN -1 ELSE 1 END) WHERE id={LockId}");
 
                 if (rowsAffected == 0)
                 {

src/Umbraco.Cms.Persistence.EFCore/Locking/SqliteEFCoreDistributedLockingMechanism.cs

@@ -16,8 +16,8 @@ namespace Umbraco.Cms.Persistence.EFCore.Locking;
 internal class SqliteEFCoreDistributedLockingMechanism<T> : IDistributedLockingMechanism
     where T : DbContext
 {
-    private readonly IOptionsMonitor<ConnectionStrings> _connectionStrings;
-    private readonly IOptionsMonitor<GlobalSettings> _globalSettings;
+    private ConnectionStrings _connectionStrings;
+    private GlobalSettings _globalSettings;
     private readonly ILogger<SqliteEFCoreDistributedLockingMechanism<T>> _logger;
     private readonly Lazy<IEFCoreScopeAccessor<T>> _efCoreScopeAccessor;
 
@@ -29,27 +29,29 @@ internal class SqliteEFCoreDistributedLockingMechanism<T> : IDistributedLockingM
     {
         _logger = logger;
         _efCoreScopeAccessor = efCoreScopeAccessor;
-        _connectionStrings = connectionStrings;
-        _globalSettings = globalSettings;
+        _globalSettings = globalSettings.CurrentValue;
+        _connectionStrings = connectionStrings.CurrentValue;
+        globalSettings.OnChange(x=>_globalSettings = x);
+        connectionStrings.OnChange(x=>_connectionStrings = x);
     }
 
     public bool HasActiveRelatedScope => _efCoreScopeAccessor.Value.AmbientScope is not null;
 
     /// <inheritdoc />
-    public bool Enabled => _connectionStrings.CurrentValue.IsConnectionStringConfigured() &&
-                           string.Equals(_connectionStrings.CurrentValue.ProviderName, "Microsoft.Data.Sqlite", StringComparison.InvariantCultureIgnoreCase) && _efCoreScopeAccessor.Value.AmbientScope is not null;
+    public bool Enabled => _connectionStrings.IsConnectionStringConfigured() &&
+                           string.Equals(_connectionStrings.ProviderName, "Microsoft.Data.Sqlite", StringComparison.InvariantCultureIgnoreCase) && _efCoreScopeAccessor.Value.AmbientScope is not null;
 
     // With journal_mode=wal we can always read a snapshot.
     public IDistributedLock ReadLock(int lockId, TimeSpan? obtainLockTimeout = null)
     {
-        obtainLockTimeout ??= _globalSettings.CurrentValue.DistributedLockingReadLockDefaultTimeout;
+        obtainLockTimeout ??= _globalSettings.DistributedLockingReadLockDefaultTimeout;
         return new SqliteDistributedLock(this, lockId, DistributedLockType.ReadLock, obtainLockTimeout.Value);
     }
 
     // With journal_mode=wal only a single write transaction can exist at a time.
     public IDistributedLock WriteLock(int lockId, TimeSpan? obtainLockTimeout = null)
     {
-        obtainLockTimeout ??= _globalSettings.CurrentValue.DistributedLockingWriteLockDefaultTimeout;
+        obtainLockTimeout ??= _globalSettings.DistributedLockingWriteLockDefaultTimeout;
         return new SqliteDistributedLock(this, lockId, DistributedLockType.WriteLock, obtainLockTimeout.Value);
     }
 

src/Umbraco.Cms.Persistence.SqlServer/Services/SqlServerDistributedLockingMechanism.cs

@@ -17,8 +17,8 @@ namespace Umbraco.Cms.Persistence.SqlServer.Services;
 /// </summary>
 public class SqlServerDistributedLockingMechanism : IDistributedLockingMechanism
 {
-    private readonly IOptionsMonitor<ConnectionStrings> _connectionStrings;
-    private readonly IOptionsMonitor<GlobalSettings> _globalSettings;
+    private ConnectionStrings _connectionStrings;
+    private GlobalSettings _globalSettings;
     private readonly ILogger<SqlServerDistributedLockingMechanism> _logger;
     private readonly Lazy<IScopeAccessor> _scopeAccessor; // Hooray it's a circular dependency.
 
@@ -33,25 +33,28 @@ public class SqlServerDistributedLockingMechanism : IDistributedLockingMechanism
     {
         _logger = logger;
         _scopeAccessor = scopeAccessor;
-        _globalSettings = globalSettings;
-        _connectionStrings = connectionStrings;
+        _globalSettings = globalSettings.CurrentValue;
+        _connectionStrings = connectionStrings.CurrentValue;
+        globalSettings.OnChange(x => _globalSettings = x);
+        connectionStrings.OnChange(x => _connectionStrings = x);
+
     }
 
     /// <inheritdoc />
-    public bool Enabled => _connectionStrings.CurrentValue.IsConnectionStringConfigured() &&
-                           string.Equals(_connectionStrings.CurrentValue.ProviderName,Constants.ProviderName, StringComparison.InvariantCultureIgnoreCase);
+    public bool Enabled => _connectionStrings.IsConnectionStringConfigured() &&
+                           string.Equals(_connectionStrings.ProviderName,Constants.ProviderName, StringComparison.InvariantCultureIgnoreCase);
 
     /// <inheritdoc />
     public IDistributedLock ReadLock(int lockId, TimeSpan? obtainLockTimeout = null)
     {
-        obtainLockTimeout ??= _globalSettings.CurrentValue.DistributedLockingReadLockDefaultTimeout;
+        obtainLockTimeout ??= _globalSettings.DistributedLockingReadLockDefaultTimeout;
         return new SqlServerDistributedLock(this, lockId, DistributedLockType.ReadLock, obtainLockTimeout.Value);
     }
 
     /// <inheritdoc />
     public IDistributedLock WriteLock(int lockId, TimeSpan? obtainLockTimeout = null)
     {
-        obtainLockTimeout ??= _globalSettings.CurrentValue.DistributedLockingWriteLockDefaultTimeout;
+        obtainLockTimeout ??= _globalSettings.DistributedLockingWriteLockDefaultTimeout;
         return new SqlServerDistributedLock(this, lockId, DistributedLockType.WriteLock, obtainLockTimeout.Value);
     }
 

src/Umbraco.Cms.Persistence.Sqlite/Services/SqliteDistributedLockingMechanism.cs

@@ -16,8 +16,8 @@ namespace Umbraco.Cms.Persistence.Sqlite.Services;
 
 public class SqliteDistributedLockingMechanism : IDistributedLockingMechanism
 {
-    private readonly IOptionsMonitor<ConnectionStrings> _connectionStrings;
-    private readonly IOptionsMonitor<GlobalSettings> _globalSettings;
+    private ConnectionStrings _connectionStrings;
+    private GlobalSettings _globalSettings;
     private readonly ILogger<SqliteDistributedLockingMechanism> _logger;
     private readonly Lazy<IScopeAccessor> _scopeAccessor;
 
@@ -29,25 +29,27 @@ public class SqliteDistributedLockingMechanism : IDistributedLockingMechanism
     {
         _logger = logger;
         _scopeAccessor = scopeAccessor;
-        _connectionStrings = connectionStrings;
-        _globalSettings = globalSettings;
+        _connectionStrings = connectionStrings.CurrentValue;
+        _globalSettings = globalSettings.CurrentValue;
+         globalSettings.OnChange(x=>_globalSettings = x);
+         connectionStrings.OnChange(x=>_connectionStrings = x);
     }
 
     /// <inheritdoc />
-    public bool Enabled => _connectionStrings.CurrentValue.IsConnectionStringConfigured() &&
-                           string.Equals(_connectionStrings.CurrentValue.ProviderName, Constants.ProviderName, StringComparison.InvariantCultureIgnoreCase);
+    public bool Enabled => _connectionStrings.IsConnectionStringConfigured() &&
+                           string.Equals(_connectionStrings.ProviderName, Constants.ProviderName, StringComparison.InvariantCultureIgnoreCase);
 
     // With journal_mode=wal we can always read a snapshot.
     public IDistributedLock ReadLock(int lockId, TimeSpan? obtainLockTimeout = null)
     {
-        obtainLockTimeout ??= _globalSettings.CurrentValue.DistributedLockingReadLockDefaultTimeout;
+        obtainLockTimeout ??= _globalSettings.DistributedLockingReadLockDefaultTimeout;
         return new SqliteDistributedLock(this, lockId, DistributedLockType.ReadLock, obtainLockTimeout.Value);
     }
 
     // With journal_mode=wal only a single write transaction can exist at a time.
     public IDistributedLock WriteLock(int lockId, TimeSpan? obtainLockTimeout = null)
     {
-        obtainLockTimeout ??= _globalSettings.CurrentValue.DistributedLockingWriteLockDefaultTimeout;
+        obtainLockTimeout ??= _globalSettings.DistributedLockingWriteLockDefaultTimeout;
         return new SqliteDistributedLock(this, lockId, DistributedLockType.WriteLock, obtainLockTimeout.Value);
     }
 

src/Umbraco.Core/Models/ContentEditing/ContentItemDisplay.cs

@@ -194,6 +194,7 @@ public ContentItemDisplay()
     ///     This is not used for outgoing model information.
     /// </remarks>
     [IgnoreDataMember]
+    [Obsolete("No longer used. Will be removed in V15.")]
     public ContentPropertyCollectionDto? ContentDto { get; set; }
 
     /// <summary>

src/Umbraco.Core/Services/UserService.cs

@@ -1,9 +1,12 @@
 using System.Data.Common;
 using System.Globalization;
 using System.Linq.Expressions;
+using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
+using Umbraco.Cms.Core.Cache;
 using Umbraco.Cms.Core.Configuration.Models;
+using Umbraco.Cms.Core.DependencyInjection;
 using Umbraco.Cms.Core.Events;
 using Umbraco.Cms.Core.Models.Membership;
 using Umbraco.Cms.Core.Notifications;
@@ -25,8 +28,11 @@ internal class UserService : RepositoryService, IUserService
     private readonly ILogger<UserService> _logger;
     private readonly IRuntimeState _runtimeState;
     private readonly IUserGroupRepository _userGroupRepository;
+    private readonly IRequestCache _requestCache;
     private readonly IUserRepository _userRepository;
 
+
+    [Obsolete("Use non-obsolete constructor. This will be removed in Umbraco 15.")]
     public UserService(
         ICoreScopeProvider provider,
         ILoggerFactory loggerFactory,
@@ -35,11 +41,26 @@ internal class UserService : RepositoryService, IUserService
         IUserRepository userRepository,
         IUserGroupRepository userGroupRepository,
         IOptions<GlobalSettings> globalSettings)
+        : this(provider, loggerFactory, eventMessagesFactory, runtimeState, userRepository, userGroupRepository, globalSettings, StaticServiceProvider.Instance.GetRequiredService<IRequestCache>())
+    {
+
+    }
+
+    public UserService(
+        ICoreScopeProvider provider,
+        ILoggerFactory loggerFactory,
+        IEventMessagesFactory eventMessagesFactory,
+        IRuntimeState runtimeState,
+        IUserRepository userRepository,
+        IUserGroupRepository userGroupRepository,
+        IOptions<GlobalSettings> globalSettings,
+        IRequestCache requestCache)
         : base(provider, loggerFactory, eventMessagesFactory)
     {
         _runtimeState = runtimeState;
         _userRepository = userRepository;
         _userGroupRepository = userGroupRepository;
+        _requestCache = requestCache;
         _globalSettings = globalSettings.Value;
         _logger = loggerFactory.CreateLogger<UserService>();
     }
@@ -1125,17 +1146,23 @@ private IEnumerable<EntityPermission> GetPermissions(IReadOnlyUserGroup[] groups
     /// <param name="path">Path to check permissions for</param>
     public EntityPermissionSet GetPermissionsForPath(IUser? user, string? path)
     {
-        var nodeIds = path?.GetIdsFromPathReversed();
-
-        if (nodeIds is null || nodeIds.Length == 0 || user is null)
+        var result = (EntityPermissionSet?)_requestCache.Get($"{nameof(GetPermissionsForPath)}|{path}|{user?.Id}", () =>
         {
-            return EntityPermissionSet.Empty();
-        }
+            var nodeIds = path?.GetIdsFromPathReversed();
+
+            if (nodeIds is null || nodeIds.Length == 0 || user is null)
+            {
+                return EntityPermissionSet.Empty();
+            }
 
-        // collect all permissions structures for all nodes for all groups belonging to the user
-        EntityPermission[] groupPermissions = GetPermissionsForPath(user.Groups.ToArray(), nodeIds, true).ToArray();
+            // collect all permissions structures for all nodes for all groups belonging to the user
+            EntityPermission[] groupPermissions = GetPermissionsForPath(user.Groups.ToArray(), nodeIds, true).ToArray();
+
+            return CalculatePermissionsForPathForUser(groupPermissions, nodeIds);
+        });
+
+        return result ?? EntityPermissionSet.Empty();
 
-        return CalculatePermissionsForPathForUser(groupPermissions, nodeIds);
     }
 
     /// <summary>

src/Umbraco.Infrastructure/Cache/PropertyEditors/BlockEditorElementTypeCache.cs

@@ -0,0 +1,32 @@
+using Umbraco.Cms.Core.Models;
+using Umbraco.Cms.Core.Services;
+using Umbraco.Extensions;
+
+namespace Umbraco.Cms.Core.Cache.PropertyEditors;
+
+internal sealed class BlockEditorElementTypeCache : IBlockEditorElementTypeCache
+{
+    private readonly IContentTypeService _contentTypeService;
+    private readonly AppCaches _appCaches;
+
+    public BlockEditorElementTypeCache(IContentTypeService contentTypeService, AppCaches appCaches)
+    {
+        _contentTypeService = contentTypeService;
+        _appCaches = appCaches;
+    }
+
+    public IEnumerable<IContentType> GetAll(IEnumerable<Guid> keys)
+    {
+        // TODO: make this less dumb; don't fetch all elements, only fetch the items that aren't yet in the cache and amend the cache as more elements are loaded
+
+        const string cacheKey = $"{nameof(BlockEditorElementTypeCache)}_ElementTypes";
+        IEnumerable<IContentType>? cachedElements = _appCaches.RequestCache.GetCacheItem<IEnumerable<IContentType>>(cacheKey);
+        if (cachedElements is null)
+        {
+            cachedElements = _contentTypeService.GetAllElementTypes();
+            _appCaches.RequestCache.Set(cacheKey, cachedElements);
+        }
+
+        return cachedElements.Where(elementType => keys.Contains(elementType.Key));
+    }
+}

src/Umbraco.Infrastructure/Cache/PropertyEditors/IBlockEditorElementTypeCache.cs

@@ -0,0 +1,8 @@
+using Umbraco.Cms.Core.Models;
+
+namespace Umbraco.Cms.Core.Cache.PropertyEditors;
+
+public interface IBlockEditorElementTypeCache
+{
+    IEnumerable<IContentType> GetAll(IEnumerable<Guid> keys);
+}

src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs

@@ -5,6 +5,7 @@
 using Serilog;
 using Umbraco.Cms.Core;
 using Umbraco.Cms.Core.Cache;
+using Umbraco.Cms.Core.Cache.PropertyEditors;
 using Umbraco.Cms.Core.Configuration;
 using Umbraco.Cms.Core.Configuration.Models;
 using Umbraco.Cms.Core.DeliveryApi;
@@ -235,6 +236,8 @@ public static IUmbracoBuilder AddCoreInitialServices(this IUmbracoBuilder builde
         builder.AddDeliveryApiCoreServices();
         builder.Services.AddTransient<IWebhookFiringService, WebhookFiringService>();
 
+        builder.Services.AddSingleton<IBlockEditorElementTypeCache, BlockEditorElementTypeCache>();
+
         return builder;
     }
 

src/Umbraco.Infrastructure/Examine/ContentValueSetBuilder.cs

@@ -128,9 +128,9 @@ public override IEnumerable<ValueSet> GetValueSets(params IContent[] content)
         // processing below instead of one by one.
         using (ICoreScope scope = _scopeProvider.CreateCoreScope())
         {
-            creatorIds = _userService.GetProfilesById(content.Select(x => x.CreatorId).ToArray())
+            creatorIds = _userService.GetProfilesById(content.Select(x => x.CreatorId).Distinct().ToArray())
                 .ToDictionary(x => x.Id, x => x);
-            writerIds = _userService.GetProfilesById(content.Select(x => x.WriterId).ToArray())
+            writerIds = _userService.GetProfilesById(content.Select(x => x.WriterId).Distinct().ToArray())
                 .ToDictionary(x => x.Id, x => x);
             scope.Complete();
         }