Skip to content

Optimize PersistRequest constructor #351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions Orm/Xtensive.Orm/Orm/Providers/Requests/ParameterBinding.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,6 @@ public abstract class ParameterBinding

public SqlExpression ParameterReference { get; }

public static IReadOnlyCollection<T> NormalizeBindings<T>(IEnumerable<T> bindings) where T : ParameterBinding =>
bindings != null ? new HashSet<T>(bindings) : Array.Empty<T>();


// Constructors

protected ParameterBinding(TypeMapping typeMapping, ParameterTransmissionType transmissionType)
Expand Down
6 changes: 4 additions & 2 deletions Orm/Xtensive.Orm/Orm/Providers/Requests/PersistRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ namespace Xtensive.Orm.Providers
/// </summary>
public sealed class PersistRequest
{
private static readonly IReadOnlySet<PersistParameterBinding> EmptyBindings = new HashSet<PersistParameterBinding>();

Choose a reason for hiding this comment

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

FrozenSet.Empty ?


private readonly StorageDriver driver;

private SqlCompilationResult compiledStatement;
Expand All @@ -43,7 +45,7 @@ public void Prepare()
// Constructors

public PersistRequest(
StorageDriver driver, SqlStatement statement, IEnumerable<PersistParameterBinding> parameterBindings)
StorageDriver driver, SqlStatement statement, IReadOnlySet<PersistParameterBinding> parameterBindings)
{
ArgumentNullException.ThrowIfNull(driver);
ArgumentNullException.ThrowIfNull(statement);
Expand All @@ -54,7 +56,7 @@ public PersistRequest(
this.driver = driver;
Statement = statement;
CompileUnit = compileUnit;
ParameterBindings = ParameterBinding.NormalizeBindings(parameterBindings);
ParameterBindings = parameterBindings ?? EmptyBindings;
}
}
}
10 changes: 5 additions & 5 deletions Orm/Xtensive.Orm/Orm/Providers/Requests/PersistRequestBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ protected virtual List<PersistRequest> BuildInsertRequest(in PersistRequestBuild
var table = context.Mapping[index.ReflectedType];
var tableRef = SqlDml.TableRef(table);
var query = SqlDml.Insert(tableRef);
var bindings = new List<PersistParameterBinding>();
var bindings = new HashSet<PersistParameterBinding>();

var row = new Dictionary<SqlColumn, SqlExpression>(index.Columns.Count);
foreach (var column in index.Columns) {
Expand All @@ -95,7 +95,7 @@ protected virtual List<PersistRequest> BuildUpdateRequest(in PersistRequestBuild
var table = context.Mapping[index.ReflectedType];
var tableRef = SqlDml.TableRef(table);
var query = SqlDml.Update(tableRef);
var bindings = new List<PersistParameterBinding>();
var bindings = new HashSet<PersistParameterBinding>();

foreach (var column in index.Columns) {
var fieldIndex = GetFieldIndex(context.Type, column);
Expand Down Expand Up @@ -137,7 +137,7 @@ protected virtual List<PersistRequest> BuildRemoveRequest(in PersistRequestBuild
var index = context.AffectedIndexes[i];
var tableRef = SqlDml.TableRef(context.Mapping[index.ReflectedType]);
var query = SqlDml.Delete(tableRef);
var bindings = new List<PersistParameterBinding>();
var bindings = new HashSet<PersistParameterBinding>();
query.Where = BuildKeyFilter(context, tableRef, bindings);
if (context.Task.ValidateVersion) {
query.Where &= BuildVersionFilter(context, tableRef, bindings);
Expand All @@ -147,7 +147,7 @@ protected virtual List<PersistRequest> BuildRemoveRequest(in PersistRequestBuild
return result;
}

private SqlExpression BuildKeyFilter(in PersistRequestBuilderContext context, SqlTableRef filteredTable, List<PersistParameterBinding> currentBindings)
private SqlExpression BuildKeyFilter(in PersistRequestBuilderContext context, SqlTableRef filteredTable, ISet<PersistParameterBinding> currentBindings)
{
SqlExpression result = null;
foreach (var column in context.PrimaryIndex.KeyColumns.Keys) {
Expand All @@ -163,7 +163,7 @@ private SqlExpression BuildKeyFilter(in PersistRequestBuilderContext context, Sq
return result;
}

private SqlExpression BuildVersionFilter(in PersistRequestBuilderContext context, SqlTableRef filteredTable, List<PersistParameterBinding> currentBindings)
private SqlExpression BuildVersionFilter(in PersistRequestBuilderContext context, SqlTableRef filteredTable, ISet<PersistParameterBinding> currentBindings)
{
SqlExpression result = null;
foreach (var column in context.Type.GetVersionColumns()) {
Expand Down
22 changes: 10 additions & 12 deletions Orm/Xtensive.Orm/Orm/Providers/Requests/QueryRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@
// Created by: Dmitri Maximov
// Created: 2008.08.22

using System;
using System.Collections.Generic;
using System.Linq;
using Xtensive.Core;
using Xtensive.Orm.Configuration;
using Xtensive.Sql.Compiler;
using Xtensive.Sql.Dml;
Expand All @@ -20,16 +16,18 @@ namespace Xtensive.Orm.Providers
/// </summary>
public sealed class QueryRequest : IQueryRequest
{
private static readonly IReadOnlySet<QueryParameterBinding> EmptyBindings = new HashSet<QueryParameterBinding>();

Choose a reason for hiding this comment

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

FrozenSet.Empty ?


private readonly StorageDriver driver;

private DbDataReaderAccessor? accessor;
private SqlCompilationResult compiledStatement;

public SqlSelect Statement { get; private set; }
public IEnumerable<QueryParameterBinding> ParameterBindings { get; private set; }
public IEnumerable<QueryParameterBinding> ParameterBindings { get; }

public TupleDescriptor TupleDescriptor { get; private set; }
public QueryRequestOptions Options { get; private set; }
public TupleDescriptor TupleDescriptor { get; }
public QueryRequestOptions Options { get; }

public NodeConfiguration NodeConfiguration { get; private set; }

Expand Down Expand Up @@ -60,7 +58,7 @@ public DbDataReaderAccessor GetAccessor() =>
// Constructors

public QueryRequest(
StorageDriver driver, SqlSelect statement, IEnumerable<QueryParameterBinding> parameterBindings,
StorageDriver driver, SqlSelect statement, IReadOnlySet<QueryParameterBinding> parameterBindings,
TupleDescriptor tupleDescriptor, QueryRequestOptions options)
{
ArgumentNullException.ThrowIfNull(driver);
Expand All @@ -69,13 +67,13 @@ public QueryRequest(

this.driver = driver;
Statement = statement;
ParameterBindings = ParameterBinding.NormalizeBindings(parameterBindings);
ParameterBindings = parameterBindings ?? EmptyBindings;
TupleDescriptor = tupleDescriptor;
Options = options;
}

public QueryRequest(
StorageDriver driver, SqlSelect statement, IEnumerable<QueryParameterBinding> parameterBindings,
StorageDriver driver, SqlSelect statement, IReadOnlySet<QueryParameterBinding> parameterBindings,
TupleDescriptor tupleDescriptor, QueryRequestOptions options, NodeConfiguration nodeConfiguration)
{
ArgumentNullException.ThrowIfNull(driver);
Expand All @@ -84,10 +82,10 @@ public QueryRequest(

this.driver = driver;
Statement = statement;
ParameterBindings = ParameterBinding.NormalizeBindings(parameterBindings);
ParameterBindings = parameterBindings ?? EmptyBindings;
TupleDescriptor = tupleDescriptor;
Options = options;
NodeConfiguration = nodeConfiguration;
}
}
}
}
10 changes: 4 additions & 6 deletions Orm/Xtensive.Orm/Orm/Providers/Requests/UserQueryRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
// Created by: Denis Krjuchkov
// Created: 2012.02.25

using System.Collections.Generic;
using Xtensive.Core;
using Xtensive.Sql.Compiler;

namespace Xtensive.Orm.Providers
Expand All @@ -19,17 +17,17 @@ public SqlCompilationResult GetCompiledStatement()
return compiledStatement;
}

public IEnumerable<QueryParameterBinding> ParameterBindings { get; private set; }
public IEnumerable<QueryParameterBinding> ParameterBindings { get; }

// Constructors

public UserQueryRequest(SqlCompilationResult compiledStatement, IEnumerable<QueryParameterBinding> parameterBindings)
public UserQueryRequest(SqlCompilationResult compiledStatement, IReadOnlySet<QueryParameterBinding> parameterBindings)
{
ArgumentNullException.ThrowIfNull(compiledStatement);
ArgumentNullException.ThrowIfNull(parameterBindings);

this.compiledStatement = compiledStatement;
ParameterBindings = ParameterBinding.NormalizeBindings(parameterBindings);
ParameterBindings = parameterBindings;
}
}
}
}
6 changes: 3 additions & 3 deletions Orm/Xtensive.Orm/Orm/Providers/SqlCompiler.Helpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ protected SqlProvider CreateProvider(SqlSelect statement, IEnumerable<QueryParam
CompilableProvider origin, params ExecutableProvider[] sources)
{
var allowBatching = true;
var parameterBindings = extraBindings ?? Enumerable.Empty<QueryParameterBinding>();
var parameterBindings = extraBindings?.ToHashSet() ?? new();
foreach (var provider in sources.OfType<SqlProvider>()) {
var queryRequest = provider.Request;
allowBatching &= queryRequest.CheckOptions(QueryRequestOptions.AllowOptimization);
parameterBindings = parameterBindings.Concat(queryRequest.ParameterBindings);
parameterBindings.UnionWith(queryRequest.ParameterBindings);
}

var tupleDescriptor = origin.Header.TupleDescriptor;
Expand Down Expand Up @@ -136,7 +136,7 @@ protected SqlExpression GetBooleanColumnExpression(SqlExpression originalExpress
: booleanExpressionConverter.BooleanToInt(originalExpression);

protected QueryRequest CreateQueryRequest(StorageDriver driver, SqlSelect statement,
IEnumerable<QueryParameterBinding> parameterBindings,
IReadOnlySet<QueryParameterBinding> parameterBindings,
TupleDescriptor tupleDescriptor, QueryRequestOptions options)
{
if (Handlers.Domain.Configuration.ShareStorageSchemaOverNodes) {
Expand Down
6 changes: 3 additions & 3 deletions Orm/Xtensive.Orm/Orm/Providers/SqlCompiler.Include.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ internal protected override SqlProvider VisitInclude(IncludeProvider provider)
var source = Compile(provider.Source);
var resultQuery = ExtractSqlSelect(provider, source);
var sourceColumns = ExtractColumnExpressions(resultQuery);
var bindings = source.Request.ParameterBindings;
var bindings = source.Request.ParameterBindings.ToHashSet();
var filterDataSource = provider.FilterDataSource.CachingCompile();
var requestOptions = QueryRequestOptions.Empty;
SqlExpression resultExpression;
Expand Down Expand Up @@ -82,8 +82,8 @@ internal protected override SqlProvider VisitInclude(IncludeProvider provider)
resultExpression = GetBooleanColumnExpression(resultExpression);
var calculatedColumn = provider.Header.Columns[provider.Header.Length - 1];
AddInlinableColumn(provider, calculatedColumn, resultQuery, resultExpression);
if (extraBinding!=null) {
bindings = bindings.Append(extraBinding);
if (extraBinding != null) {
bindings.Add(extraBinding);
}

var request = CreateQueryRequest(Driver, resultQuery, bindings, provider.Header.TupleDescriptor, requestOptions);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public TemporaryTableDescriptor BuildDescriptor(ModelMapping modelMapping, strin
Lazy<PersistRequest> CreateLazyPersistRequest(ushort batchSize)
{
return new Lazy<PersistRequest>(() => {
var bindings = new List<PersistParameterBinding>(batchSize);
var bindings = new HashSet<PersistParameterBinding>(batchSize);
var statement = MakeUpInsertQuery(tableRef, typeMappings, bindings, hasColumns, batchSize);
var persistRequest = new PersistRequest(driver, statement, bindings);
persistRequest.Prepare();
Expand Down Expand Up @@ -214,7 +214,7 @@ private SqlSelect MakeUpSelectQuery(SqlTableRef temporaryTable, bool hasColumns)
}

private SqlInsert MakeUpInsertQuery(SqlTableRef temporaryTable,
TypeMapping[] typeMappings, List<PersistParameterBinding> storeRequestBindings, bool hasColumns, ushort rows = 1)
TypeMapping[] typeMappings, ISet<PersistParameterBinding> storeRequestBindings, bool hasColumns, ushort rows = 1)
{
var insertStatement = SqlDml.Insert(temporaryTable);
if (!hasColumns) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public QueryRequest CreateRequest(SqlCompilationResult compiledQuery, IEnumerabl

return new QueryRequest(new UserQueryRequest(
compiledQuery,
bindings.Select(b => b.RealBinding)));
bindings.Select(b => b.RealBinding).ToHashSet()));
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,12 @@ private IPersistDescriptor CreateDescriptor(string tableName,
var tableRef = SqlDml.TableRef(table);

var insert = SqlDml.Insert(tableRef);
var bindings = new PersistParameterBinding[columns.Count];
var bindings = new HashSet<PersistParameterBinding>(columns.Count);
var row = new Dictionary<SqlColumn, SqlExpression>(columns.Count);
for (ColNum i = 0; i < columns.Count; i++) {
var binding = new PersistParameterBinding(mappings[i], i, transmissionTypes[i]);
row.Add(tableRef.Columns[i], binding.ParameterReference);
bindings[i] = binding;
bindings.Add(binding);
}
insert.ValueRows.Add(row);

Expand Down
7 changes: 1 addition & 6 deletions Orm/Xtensive.Orm/Sql/Compiler/SqlCompilationResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ public sealed class SqlCompilationResult
private readonly IReadOnlyDictionary<object, string> parameterNames;
private volatile int lastResultLength;

private readonly TypeIdRegistry typeIdRegistry;

/// <inheritdoc/>
public override string ToString()
{
Expand Down Expand Up @@ -75,9 +73,7 @@ public string GetCommandText(SqlPostCompilerConfiguration configuration)
// Constructors

internal SqlCompilationResult(IReadOnlyList<Node> result,
IReadOnlyDictionary<object, string> parameterNames,
TypeIdRegistry typeIdRegistry
)
IReadOnlyDictionary<object, string> parameterNames)
{
switch (result.Count) {
case 0:
Expand All @@ -91,7 +87,6 @@ TypeIdRegistry typeIdRegistry
break;
}
this.parameterNames = parameterNames.Count > 0 ? parameterNames : null;
this.typeIdRegistry = typeIdRegistry;
}
}
}
2 changes: 1 addition & 1 deletion Orm/Xtensive.Orm/Sql/Compiler/SqlCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public SqlCompilationResult Compile(ISqlCompileUnit unit, SqlCompilerConfigurati
configuration = compilerConfiguration;
context = new SqlCompilerContext(configuration);
unit.AcceptVisitor(this);
return new SqlCompilationResult(context.Output.Children, context.ParameterNameProvider.NameTable, typeIdRegistry);
return new SqlCompilationResult(context.Output.Children, context.ParameterNameProvider.NameTable);
}

#region Visitors
Expand Down
4 changes: 2 additions & 2 deletions Orm/Xtensive.Orm/Sql/SqlDriver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public abstract class SqlDriver
public SqlCompilationResult Compile(ISqlCompileUnit statement)
{
ArgumentNullException.ThrowIfNull(statement);
return CreateCompiler().Compile(statement, new SqlCompilerConfiguration(), null);
return CreateCompiler().Compile(statement, new SqlCompilerConfiguration());
}

/// <summary>
Expand All @@ -77,7 +77,7 @@ public SqlCompilationResult Compile(ISqlCompileUnit statement, SqlCompilerConfig
ArgumentNullException.ThrowIfNull(statement);
ArgumentNullException.ThrowIfNull(configuration);
ValidateCompilerConfiguration(configuration);
return CreateCompiler().Compile(statement, configuration, typeIdRegistry);
return CreateCompiler().Compile(statement, configuration);
}

/// <summary>
Expand Down