Skip to content

Commit 21c60ba

Browse files
author
fabien.menager
committed
Various code smells and optimizations: 20MB less allocations on the Postgresql and SQLite benchmarks with 500K rows thanks to for instead of foreach
1 parent 0e0d1aa commit 21c60ba

7 files changed

Lines changed: 48 additions & 43 deletions

File tree

‎.editorconfig‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,5 +59,8 @@ dotnet_style_predefined_type_for_member_access = true:suggestion
5959
# IDE0090: Use 'new(...)'
6060
dotnet_diagnostic.IDE0090.severity = none
6161

62+
# Resharper
63+
resharper_for_can_be_converted_to_foreach_highlighting = none
64+
6265
[*.{csproj,proj,targets}]
6366
indent_size = 2

‎src/PhenX.EntityFrameworkCore.BulkInsert.MySql/MySqlBulkInsertProvider.cs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ protected override IAsyncEnumerable<T> BulkInsertReturnEntities<T>(
3434
TableMetadata tableInfo,
3535
IEnumerable<T> entities,
3636
MySqlBulkInsertOptions options,
37-
OnConflictOptions<T>? onConflict = null,
38-
CancellationToken ctk = default)
37+
OnConflictOptions<T>? onConflict,
38+
CancellationToken ctk)
3939
{
4040
throw new NotSupportedException("Provider does not support returning entities.");
4141
}
@@ -71,14 +71,16 @@ CancellationToken ctk
7171
sourceOrdinal++;
7272
}
7373

74+
var dataReader = new EnumerableDataReader<T>(entities, properties, options.Converters);
75+
7476
if (sync)
7577
{
7678
// ReSharper disable once MethodHasAsyncOverloadWithCancellation
77-
bulkCopy.WriteToServer(new EnumerableDataReader<T>(entities, properties, options.Converters));
79+
bulkCopy.WriteToServer(dataReader);
7880
}
7981
else
8082
{
81-
await bulkCopy.WriteToServerAsync(new EnumerableDataReader<T>(entities, properties, options.Converters), ctk);
83+
await bulkCopy.WriteToServerAsync(dataReader, ctk);
8284
}
8385
}
8486
}

‎src/PhenX.EntityFrameworkCore.BulkInsert.PostgreSql/PostgreSqlBulkInsertProvider.cs‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ protected override async Task BulkInsert<T>(
5757
? connection.BeginBinaryImport(command)
5858
: await connection.BeginBinaryImportAsync(command, ctk);
5959

60+
var bulkValueConverters = options.Converters;
61+
6062
// The type mapping can be null for obvious types like string.
6163
var columnTypes = columns.Select(c => GetPostgreSqlType(c, options)).ToArray();
6264

@@ -72,10 +74,9 @@ protected override async Task BulkInsert<T>(
7274
await writer.StartRowAsync(ctk);
7375
}
7476

75-
var columnIndex = 0;
76-
foreach (var column in columns)
77+
for (var columnIndex = 0; columnIndex < columns.Count; columnIndex++)
7778
{
78-
var value = column.GetValue(entity, options.Converters);
79+
var value = columns[columnIndex].GetValue(entity, bulkValueConverters);
7980

8081
// Get the actual type, so that the writer can do the conversation to the target type automatically.
8182
var type = columnTypes[columnIndex];
@@ -104,8 +105,6 @@ protected override async Task BulkInsert<T>(
104105
await writer.WriteAsync(value, ctk);
105106
}
106107
}
107-
108-
columnIndex++;
109108
}
110109
}
111110

@@ -126,7 +125,7 @@ protected override async Task BulkInsert<T>(
126125
private static NpgsqlDbType? GetPostgreSqlType(ColumnMetadata column, PostgreSqlBulkInsertOptions options)
127126
{
128127
var typeProviders = options.TypeProviders;
129-
if (typeProviders is { Count: > 0 })
128+
if (typeProviders is { Count: > 0 })
130129
{
131130
foreach (var typeProvider in typeProviders)
132131
{

‎src/PhenX.EntityFrameworkCore.BulkInsert.SqlServer/SqlServerBulkInsertProvider.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,16 @@ protected override async Task BulkInsert<T>(
5151
bulkCopy.ColumnMappings.Add(column.PropertyName, column.ColumnName);
5252
}
5353

54+
var dataReader = new EnumerableDataReader<T>(entities, columns, options.Converters);
55+
5456
if (sync)
5557
{
5658
// ReSharper disable once MethodHasAsyncOverloadWithCancellation
57-
bulkCopy.WriteToServer(new EnumerableDataReader<T>(entities, columns, options.Converters));
59+
bulkCopy.WriteToServer(dataReader);
5860
}
5961
else
6062
{
61-
await bulkCopy.WriteToServerAsync(new EnumerableDataReader<T>(entities, columns, options.Converters), ctk);
63+
await bulkCopy.WriteToServerAsync(dataReader, ctk);
6264
}
6365
}
6466
}

‎src/PhenX.EntityFrameworkCore.BulkInsert.SqlServer/SqlServerDialectBuilder.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,19 +75,19 @@ public override string BuildMoveDataSql<T>(
7575
q.AppendLine(")");
7676

7777
q.Append("ON ");
78-
q.AppendJoin($" AND ", matchColumns, (b, col) => b.Append($"TARGET.{col} = SOURCE.{col}"));
78+
q.AppendJoin(" AND ", matchColumns, (b, col) => b.Append($"TARGET.{col} = SOURCE.{col}"));
7979
q.AppendLine();
8080

8181
if (onConflictTyped.Update != null)
8282
{
8383
var columns = target.GetColumns(false);
8484

85-
q.AppendLine($"WHEN MATCHED THEN UPDATE SET ");
85+
q.AppendLine("WHEN MATCHED THEN UPDATE SET ");
8686
q.AppendJoin(", ", GetUpdates(target, columns, onConflictTyped.Update));
8787
q.AppendLine();
8888
}
8989

90-
q.Append($"WHEN NOT MATCHED THEN INSERT (");
90+
q.Append("WHEN NOT MATCHED THEN INSERT (");
9191
q.AppendColumns(insertedColumns);
9292
q.AppendLine(")");
9393

@@ -98,7 +98,7 @@ public override string BuildMoveDataSql<T>(
9898
if (returnedColumns.Count != 0)
9999
{
100100
q.Append("OUTPUT ");
101-
q.AppendJoin($", ", returnedColumns, (b, col) => b.Append($"INSERTED.{col.QuotedColumName} AS {col.QuotedColumName}"));
101+
q.AppendJoin(", ", returnedColumns, (b, col) => b.Append($"INSERTED.{col.QuotedColumName} AS {col.QuotedColumName}"));
102102
q.AppendLine();
103103
}
104104
}
@@ -113,7 +113,7 @@ public override string BuildMoveDataSql<T>(
113113
if (returnedColumns.Count != 0)
114114
{
115115
q.Append("OUTPUT ");
116-
q.AppendJoin($", ", returnedColumns, (b, col) => b.Append($"INSERTED.{col.QuotedColumName} AS {col.QuotedColumName}"));
116+
q.AppendJoin(", ", returnedColumns, (b, col) => b.Append($"INSERTED.{col.QuotedColumName} AS {col.QuotedColumName}"));
117117
q.AppendLine();
118118
}
119119

‎src/PhenX.EntityFrameworkCore.BulkInsert.Sqlite/SqliteBulkInsertProvider.cs‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -46,22 +46,23 @@ private static SqliteType GetSqliteType(ColumnMetadata column)
4646
{
4747
return SqliteType.Integer;
4848
}
49-
else if (string.Equals(storeType, "FLOAT", StringComparison.OrdinalIgnoreCase))
49+
50+
if (string.Equals(storeType, "FLOAT", StringComparison.OrdinalIgnoreCase))
5051
{
5152
return SqliteType.Real;
5253
}
53-
else if (string.Equals(storeType, "TEXT", StringComparison.OrdinalIgnoreCase))
54+
55+
if (string.Equals(storeType, "TEXT", StringComparison.OrdinalIgnoreCase))
5456
{
5557
return SqliteType.Text;
5658
}
57-
else if (string.Equals(storeType, "BLOB", StringComparison.OrdinalIgnoreCase))
59+
60+
if (string.Equals(storeType, "BLOB", StringComparison.OrdinalIgnoreCase))
5861
{
5962
return SqliteType.Blob;
6063
}
61-
else
62-
{
63-
throw new NotSupportedException($"Invalid store type '{storeType}' for property '{column.PropertyName}'");
64-
}
64+
65+
throw new NotSupportedException($"Invalid store type '{storeType}' for property '{column.PropertyName}'");
6566
}
6667

6768
private static DbCommand GetInsertCommand(
@@ -91,7 +92,7 @@ private static DbCommand GetInsertCommand(
9192
sb.Append('(');
9293

9394
var columnIndex = 0;
94-
foreach (var column in columns)
95+
for (var index = 0; index < columns.Count; index++)
9596
{
9697
var parameterName = $"@p{p++}";
9798
command.Parameters.Add(new SqliteParameter(parameterName, columnTypes[columnIndex]));
@@ -129,7 +130,7 @@ CancellationToken ctk
129130
{
130131
var batchSize = Math.Min(options.BatchSize, MaxParams / columns.Count);
131132

132-
// The StringBuilder can be resuse between the batches.
133+
// The StringBuilder can be reused between the batches.
133134
var sb = new StringBuilder();
134135

135136
var columnList = tableInfo.GetColumns(options.CopyGeneratedColumns);
@@ -186,10 +187,14 @@ private static async Task ExecuteCommand(bool sync, DbCommand insertCommand, Can
186187
private static void FillValues<T>(T[] chunk, DbParameterCollection parameters, IReadOnlyList<ColumnMetadata> columns) where T : class
187188
{
188189
var p = 0;
189-
foreach (var entity in chunk)
190+
191+
for (var chunkIndex = 0; chunkIndex < chunk.Length; chunkIndex++)
190192
{
191-
foreach (var column in columns)
193+
var entity = chunk[chunkIndex];
194+
195+
for (var columnIndex = 0; columnIndex < columns.Count; columnIndex++)
192196
{
197+
var column = columns[columnIndex];
193198
var value = column.GetValue(entity, null);
194199
parameters[p].Value = value;
195200
p++;

‎src/PhenX.EntityFrameworkCore.BulkInsert/Metadata/ColumnMetadata.cs‎

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,11 @@ internal sealed class ColumnMetadata(IProperty property, SqlDialectBuilder dial
2222

2323
public Type ClrType { get; } = property.ClrType;
2424

25-
public Type? ProviderClrType { get; } = property.GetProviderClrType();
26-
2725
public bool IsGenerated { get; } = property.ValueGenerated == ValueGenerated.OnAdd;
2826

2927
public object? GetValue(object entity, List<IBulkValueConverter>? converters)
3028
{
31-
var result = _getter(entity!);
29+
var result = _getter(entity);
3230

3331
if (converters != null && result != null)
3432
{
@@ -51,26 +49,22 @@ internal sealed class ColumnMetadata(IProperty property, SqlDialectBuilder dial
5149
property.GetValueConverter() ??
5250
property.GetTypeMapping().Converter;
5351

52+
var propInfo = property.PropertyInfo!;
53+
5454
var actualGetter =
5555
PropertyAccessor.CreateUntypedGetter(
56-
property.PropertyInfo!,
56+
propInfo,
5757
property.DeclaringType.ClrType,
5858
property.ClrType);
5959

60-
var result = actualGetter;
61-
if (valueConverter != null)
60+
if (valueConverter == null)
6261
{
63-
var converter = valueConverter.ConvertToProvider;
64-
65-
result = source =>
66-
{
67-
var value = actualGetter(source);
68-
69-
return converter(value);
70-
};
62+
return actualGetter;
7163
}
7264

73-
return result;
65+
var converter = valueConverter.ConvertToProvider;
66+
67+
return source => converter(actualGetter(source));
7468
}
7569

7670
private static string GetStoreDefinition(IProperty property)

0 commit comments

Comments
 (0)