Skip to content

Commit

Permalink
Nonbreaking performance tweaks (#17106)
Browse files Browse the repository at this point in the history
* Avoid doing multiple lookups in dictionaries, avoid doing string interpolation & adding single char strings to a StringBuilder, made some private/internal classes & some private methods static when possible, use FrozenSet for InvalidFileNameChars

* Avoid some array + list allocations & async methods and made some private methods static

* Avoid double lookup of XML attribute (and double null check) & avoid an unneeded lookup before writing to a dictionary

* Avoid some double lookups

# Conflicts:
#	src/Umbraco.Core/Services/LocalizedTextService.cs

* Avoid double lookups

# Conflicts:
#	src/Umbraco.Core/Services/LocalizedTextService.cs

* Avoid double lookups

* List AsSpan, also to trigger a new build that hopefully goes through

* Avoid concatting strings when using writer & more static

* Updated CollectionBenchmarks to show that ToArray isn't always the fastest & Lists can be iterated nearly as fast as arrays (and that ToList is nearly as fast as ToArray on IReadOnlyLists in .NET 8)

* Fix rebase

* Use explicit types ❤️ (I thought it was the other way round...)

# Conflicts:
#	src/Umbraco.Core/Services/LocalizedTextService.cs

* Reduce number of lines in HtmlStringUtilities.Truncate to pass code quality analysis

* Avoid double lookups & allocating empty arrays

* Use native List Find instead of LINQ
  • Loading branch information
Henr1k80 authored Jan 31, 2025
1 parent 55b5d7e commit c64ec51
Show file tree
Hide file tree
Showing 69 changed files with 443 additions and 442 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@ public FilterOption BuildFilterOption(string filter)
{
GroupCollection groups = QueryParserRegex.Match(filter).Groups;

if (groups.Count != 3 || groups.ContainsKey("operator") is false || groups.ContainsKey("value") is false)
if (groups.Count != 3 || groups.TryGetValue("operator", out Group? operatorGroup) is false || groups.TryGetValue("value", out Group? valueGroup) is false)
{
return DefaultFilterOption();
}

FilterOperation? filterOperation = ParseFilterOperation(groups["operator"].Value);
FilterOperation? filterOperation = ParseFilterOperation(operatorGroup.Value);
if (filterOperation.HasValue is false)
{
return DefaultFilterOption();
Expand All @@ -51,7 +51,7 @@ public FilterOption BuildFilterOption(string filter)
return new FilterOption
{
FieldName = FieldName,
Values = new[] { groups["value"].Value },
Values = [valueGroup.Value],
Operator = filterOperation.Value
};

Expand Down
12 changes: 8 additions & 4 deletions src/Umbraco.Cms.Api.Delivery/Services/ApiContentQueryProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,14 @@ public PagedModel<Guid> ExecuteQuery(
return new PagedModel<Guid>();
}

Guid[] items = results
.Where(r => r.Values.ContainsKey(ItemIdFieldName))
.Select(r => Guid.Parse(r.Values[ItemIdFieldName]))
.ToArray();
List<Guid> items = [];
foreach (ISearchResult result in results)
{
if (result.Values.TryGetValue(ItemIdFieldName, out string? value))
{
items.Add(Guid.Parse(value));
}
}

return new PagedModel<Guid>(results.TotalItemCount, items);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ private IEnumerable<IPublishedContent> BuildQuery(TemplateQueryExecuteModel mode
if (model.RootDocument?.Id is not null)
{
rootContent = _publishedContentQuery.Content(model.RootDocument.Id);
queryExpression.Append($"Umbraco.Content(Guid.Parse(\"{model.RootDocument.Id}\"))");
queryExpression.Append("Umbraco.Content(Guid.Parse(\"").Append(model.RootDocument.Id).Append("\"))");
}
else
{
Expand All @@ -115,7 +115,7 @@ private IEnumerable<IPublishedContent> GetRootContentQuery(TemplateQueryExecuteM

if (model.DocumentTypeAlias.IsNullOrWhiteSpace() == false)
{
queryExpression.Append($".ChildrenOfType(\"{model.DocumentTypeAlias}\")");
queryExpression.Append(".ChildrenOfType(\"").Append(model.DocumentTypeAlias).Append("\")");
return rootContent == null
? Enumerable.Empty<IPublishedContent>()
: rootContent.ChildrenOfType(_variationContextAccessor, _contentCache, _documentNavigationQueryService, model.DocumentTypeAlias);
Expand Down Expand Up @@ -176,7 +176,7 @@ string PropertyModelType(TemplateQueryPropertyType templateQueryPropertyType)
//for review - this uses a tonized query rather then the normal linq query.
contentQuery = contentQuery.Where(operation.Compile());
queryExpression.Append(_indent);
queryExpression.Append($".Where({operation})");
queryExpression.Append(".Where(").Append(operation).Append(')');
}

return contentQuery;
Expand Down Expand Up @@ -220,7 +220,7 @@ private IEnumerable<IPublishedContent> ApplySorting(TemplateQueryExecuteSortMode
return contentQuery;
}

private IEnumerable<IPublishedContent> ApplyPaging(int take, IEnumerable<IPublishedContent> contentQuery, StringBuilder queryExpression)
private static IEnumerable<IPublishedContent> ApplyPaging(int take, IEnumerable<IPublishedContent> contentQuery, StringBuilder queryExpression)
{
if (take <= 0)
{
Expand All @@ -229,7 +229,7 @@ private IEnumerable<IPublishedContent> ApplyPaging(int take, IEnumerable<IPublis

contentQuery = contentQuery.Take(take);
queryExpression.Append(_indent);
queryExpression.Append($".Take({take})");
queryExpression.Append(".Take(").Append(take).Append(')');

return contentQuery;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ protected IEnumerable<TPropertyTypeModel> MapPropertyTypes(TContentType source)
{
Id = propertyType.Key,
SortOrder = propertyType.SortOrder,
Container = groupKeysByPropertyKeys.ContainsKey(propertyType.Key)
? new ReferenceByIdModel(groupKeysByPropertyKeys[propertyType.Key])
Container = groupKeysByPropertyKeys.TryGetValue(propertyType.Key, out Guid groupKey)
? new ReferenceByIdModel(groupKey)
: null,
Name = propertyType.Name,
Alias = propertyType.Alias,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Reflection;
using System.Runtime.InteropServices;
using Umbraco.Cms.Api.Management.Controllers;
using Umbraco.Cms.Core.Composing;
using Umbraco.Cms.Core.Services;
Expand All @@ -19,7 +20,7 @@ public class ConflictingRouteService : IConflictingRouteService
public bool HasConflictingRoutes(out string controllerName)
{
var controllers = _typeLoader.GetTypes<ManagementApiControllerBase>().ToList();
foreach (Type controller in controllers)
foreach (Type controller in CollectionsMarshal.AsSpan(controllers))
{
Type[] potentialConflicting = controllers.Where(x => x.Name == controller.Name).ToArray();
if (potentialConflicting.Length > 1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

namespace Umbraco.Cms.Persistence.EFCore.Locking;

internal class SqlServerEFCoreDistributedLockingMechanism<T> : IDistributedLockingMechanism
internal sealed class SqlServerEFCoreDistributedLockingMechanism<T> : IDistributedLockingMechanism
where T : DbContext
{
private ConnectionStrings _connectionStrings;
Expand Down Expand Up @@ -58,7 +58,7 @@ public IDistributedLock WriteLock(int lockId, TimeSpan? obtainLockTimeout = null
return new SqlServerDistributedLock(this, lockId, DistributedLockType.WriteLock, obtainLockTimeout.Value);
}

private class SqlServerDistributedLock : IDistributedLock
private sealed class SqlServerDistributedLock : IDistributedLock
{
private readonly SqlServerEFCoreDistributedLockingMechanism<T> _parent;
private readonly TimeSpan _timeout;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

namespace Umbraco.Cms.Persistence.EFCore.Locking;

internal class SqliteEFCoreDistributedLockingMechanism<T> : IDistributedLockingMechanism
internal sealed class SqliteEFCoreDistributedLockingMechanism<T> : IDistributedLockingMechanism
where T : DbContext
{
private ConnectionStrings _connectionStrings;
Expand Down Expand Up @@ -55,7 +55,7 @@ public IDistributedLock WriteLock(int lockId, TimeSpan? obtainLockTimeout = null
return new SqliteDistributedLock(this, lockId, DistributedLockType.WriteLock, obtainLockTimeout.Value);
}

private class SqliteDistributedLock : IDistributedLock
private sealed class SqliteDistributedLock : IDistributedLock
{
private readonly SqliteEFCoreDistributedLockingMechanism<T> _parent;
private readonly TimeSpan _timeout;
Expand Down Expand Up @@ -164,7 +164,7 @@ private void ObtainWriteLock()
});
}

private bool IsBusyOrLocked(SqliteException ex) =>
private static bool IsBusyOrLocked(SqliteException ex) =>
ex.SqliteErrorCode
is raw.SQLITE_BUSY
or raw.SQLITE_LOCKED
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

namespace Umbraco.Cms.Persistence.EFCore.Scoping;

internal class EFCoreDetachableScope<TDbContext> : EFCoreScope<TDbContext> where TDbContext : DbContext
internal sealed class EFCoreDetachableScope<TDbContext> : EFCoreScope<TDbContext> where TDbContext : DbContext
{
private readonly IEFCoreScopeAccessor<TDbContext> _efCoreScopeAccessor;
private readonly EFCoreScopeProvider<TDbContext> _efCoreScopeProvider;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

namespace Umbraco.Cms.Persistence.EFCore.Scoping;

internal class EFCoreScopeAccessor<TDbContext> : IEFCoreScopeAccessor<TDbContext> where TDbContext : DbContext
internal sealed class EFCoreScopeAccessor<TDbContext> : IEFCoreScopeAccessor<TDbContext> where TDbContext : DbContext
{
private readonly IAmbientEFCoreScopeStack<TDbContext> _ambientEfCoreScopeStack;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

namespace Umbraco.Cms.Persistence.EFCore.Scoping;

internal class EFCoreScopeProvider<TDbContext> : IEFCoreScopeProvider<TDbContext> where TDbContext : DbContext
internal sealed class EFCoreScopeProvider<TDbContext> : IEFCoreScopeProvider<TDbContext> where TDbContext : DbContext
{
private readonly IAmbientEFCoreScopeStack<TDbContext> _ambientEfCoreScopeStack;
private readonly ILoggerFactory _loggerFactory;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

namespace Umbraco.Cms.Persistence.SqlServer.Dtos;

internal class ColumnInSchemaDto
internal sealed class ColumnInSchemaDto
{
[Column("TABLE_NAME")]
public string TableName { get; set; } = null!;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

namespace Umbraco.Cms.Persistence.SqlServer.Dtos;

internal class ConstraintPerColumnDto
internal sealed class ConstraintPerColumnDto
{
[Column("TABLE_NAME")]
public string TableName { get; set; } = null!;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

namespace Umbraco.Cms.Persistence.SqlServer.Dtos;

internal class ConstraintPerTableDto
internal sealed class ConstraintPerTableDto
{
[Column("TABLE_NAME")]
public string TableName { get; set; } = null!;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

namespace Umbraco.Cms.Persistence.SqlServer.Dtos;

internal class DefaultConstraintPerColumnDto
internal sealed class DefaultConstraintPerColumnDto
{
[Column("TABLE_NAME")]
public string TableName { get; set; } = null!;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

namespace Umbraco.Cms.Persistence.SqlServer.Dtos;

internal class DefinedIndexDto
internal sealed class DefinedIndexDto
{
[Column("TABLE_NAME")]
public string TableName { get; set; } = null!;
Expand Down
2 changes: 1 addition & 1 deletion src/Umbraco.Cms.Persistence.SqlServer/LocalDb.cs
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,7 @@ private static void SetCommand(SqlCommand cmd, string sql, params object[] args)
/// <param name="ldfName">The LDF logical name.</param>
/// <param name="mdfFilename">The MDF filename.</param>
/// <param name="ldfFilename">The LDF filename.</param>
private void GetFilenames(
private static void GetFilenames(
SqlCommand cmd,
string databaseName,
out string? mdfName,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Data;
using System.Runtime.InteropServices;
using Microsoft.Extensions.Logging;
using NPoco;
using Umbraco.Cms.Core;
Expand Down Expand Up @@ -36,7 +37,7 @@ protected MicrosoftSqlSyntaxProviderBase()

public override string GetQuotedTableName(string? tableName)
{
if (tableName?.Contains(".") == false)
if (tableName?.Contains('.') == false)
{
return $"[{tableName}]";
}
Expand Down Expand Up @@ -207,14 +208,14 @@ public override void HandleCreateTable(IDatabase database, TableDefinition table

List<string> indexSql = Format(tableDefinition.Indexes);
//Loop through index statements and execute sql
foreach (var sql in indexSql)
foreach (var sql in CollectionsMarshal.AsSpan(indexSql))
{
_logger.LogInformation("Create Index:\n {Sql}", sql);
database.Execute(new Sql(sql));
}

//Loop through foreignkey statements and execute sql
foreach (var sql in foreignSql)
foreach (var sql in CollectionsMarshal.AsSpan(foreignSql))
{
_logger.LogInformation("Create Foreign Key:\n {Sql}", sql);
database.Execute(new Sql(sql));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace Umbraco.Cms.Persistence.SqlServer.Services;
/// this
/// reader.
/// </remarks>
internal class PocoDataDataReader<T, TSyntax> : BulkDataReader
internal sealed class PocoDataDataReader<T, TSyntax> : BulkDataReader
where TSyntax : ISqlSyntaxProvider
{
private readonly ColumnDefinition[] _columnDefinitions;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,20 +83,20 @@ public string GenerateConnectionString(DatabaseModel databaseModel)
var user = databaseModel.Login;
var password = databaseModel.Password;

if (server.Contains(".") && ServerStartsWithTcp(server) == false)
if (server.Contains('.') && ServerStartsWithTcp(server) == false)
{
server = $"tcp:{server}";
}

if (server.Contains(".") == false && ServerStartsWithTcp(server))
if (server.Contains('.') == false && ServerStartsWithTcp(server))
{
var serverName = server.Contains(",")
var serverName = server.Contains(',')
? server.Substring(0, server.IndexOf(",", StringComparison.Ordinal))
: server;

var portAddition = string.Empty;

if (server.Contains(","))
if (server.Contains(','))
{
portAddition = server.Substring(server.IndexOf(",", StringComparison.Ordinal));
}
Expand All @@ -109,12 +109,12 @@ public string GenerateConnectionString(DatabaseModel databaseModel)
server = $"tcp:{server}.database.windows.net";
}

if (server.Contains(",") == false)
if (server.Contains(',') == false)
{
server = $"{server},1433";
}

if (user?.Contains("@") == false)
if (user?.Contains('@') == false)
{
var userDomain = server;

Expand All @@ -123,7 +123,7 @@ public string GenerateConnectionString(DatabaseModel databaseModel)
userDomain = userDomain.Substring(userDomain.IndexOf(":", StringComparison.Ordinal) + 1);
}

if (userDomain.Contains("."))
if (userDomain.Contains('.'))
{
userDomain = userDomain.Substring(0, userDomain.IndexOf(".", StringComparison.Ordinal));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public int BulkInsertRecords<T>(IUmbracoDatabase database, IEnumerable<T> record
/// <param name="pocoData">The PocoData object corresponding to the record's type.</param>
/// <param name="records">The records.</param>
/// <returns>The number of records that were inserted.</returns>
private int BulkInsertRecordsSqlServer<T>(IUmbracoDatabase database, PocoData pocoData, IEnumerable<T> records)
private static int BulkInsertRecordsSqlServer<T>(IUmbracoDatabase database, PocoData pocoData, IEnumerable<T> records)
{
// TODO: The main reason this exists is because the NPoco InsertBulk method doesn't return the number of items.
// It is worth investigating the performance of this vs NPoco's because we use a custom BulkDataReader
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public string GenerateConnectionString(DatabaseModel databaseModel)
return connectionString;
}

private string HandleIntegratedAuthentication(string connectionString, DatabaseModel databaseModel)
private static string HandleIntegratedAuthentication(string connectionString, DatabaseModel databaseModel)
{
if (databaseModel.IntegratedAuth)
{
Expand All @@ -96,7 +96,7 @@ private string HandleIntegratedAuthentication(string connectionString, DatabaseM
return connectionString;
}

private string HandleTrustServerCertificate(string connectionString, DatabaseModel databaseModel)
private static string HandleTrustServerCertificate(string connectionString, DatabaseModel databaseModel)
{
if (databaseModel.TrustServerCertificate)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public IDistributedLock WriteLock(int lockId, TimeSpan? obtainLockTimeout = null
return new SqlServerDistributedLock(this, lockId, DistributedLockType.WriteLock, obtainLockTimeout.Value);
}

private class SqlServerDistributedLock : IDistributedLock
private sealed class SqlServerDistributedLock : IDistributedLock
{
private readonly SqlServerDistributedLockingMechanism _parent;
private readonly TimeSpan _timeout;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ public ServerVersionInfo(string edition, string instanceName, string productVers
private static SqlInspectionUtilities SqlInspector =>
_sqlInspector ??= new SqlInspectionUtilities();

private class SqlInspectionUtilities
private sealed class SqlInspectionUtilities
{
private readonly Func<Sql, Sql> _getSqlRhs;
private readonly Func<Sql, string> _getSqlText;
Expand All @@ -463,7 +463,7 @@ public SqlInspectionUtilities()

#endregion

private class SqlPrimaryKey
private sealed class SqlPrimaryKey
{
public string Name { get; set; } = null!;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public int BulkInsertRecords<T>(IUmbracoDatabase database, IEnumerable<T> record
/// <param name="pocoData">The PocoData object corresponding to the record's type.</param>
/// <param name="records">The records.</param>
/// <returns>The number of records that were inserted.</returns>
private int BulkInsertRecordsSqlite<T>(IUmbracoDatabase database, PocoData pocoData, IEnumerable<T> records)
private static int BulkInsertRecordsSqlite<T>(IUmbracoDatabase database, PocoData pocoData, IEnumerable<T> records)
{
var count = 0;
var inTrans = database.InTransaction;
Expand Down
Loading

0 comments on commit c64ec51

Please sign in to comment.