Skip to content

Commit f534b27

Browse files
authored
Merge pull request #15 from servicetitan/keyexpression
KeyExpression performance improvements
2 parents a7749e2 + d8a786f commit f534b27

2 files changed

Lines changed: 84 additions & 56 deletions

File tree

Orm/Xtensive.Orm/Orm/Linq/Expressions/FieldExpression.cs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,32 +18,36 @@ internal class FieldExpression : PersistentFieldExpression
1818
{
1919
private IPersistentExpression owner;
2020

21-
public new FieldInfo Field { get; private set; }
21+
public new FieldInfo Field { get; }
2222

2323
public virtual IPersistentExpression Owner
2424
{
25-
get { return owner; }
25+
get => owner;
2626
internal set
2727
{
28-
if (owner!=null)
28+
if (owner!=null) {
2929
throw Exceptions.AlreadyInitialized("Owner");
30+
}
31+
3032
owner = value;
3133
}
3234
}
3335

3436
public override Expression Remap(int offset, Dictionary<Expression, Expression> processedExpressions)
3537
{
36-
if (!CanRemap)
38+
if (!CanRemap) {
3739
return this;
40+
}
3841

39-
Expression result;
40-
if (processedExpressions.TryGetValue(this, out result))
42+
if (processedExpressions.TryGetValue(this, out var result)) {
4143
return result;
44+
}
4245

4346
var mapping = new Segment<int>(Mapping.Offset + offset, Mapping.Length);
4447
result = new FieldExpression(ExtendedExpressionType.Field, Field, mapping, OuterParameter, DefaultIfEmpty);
45-
if (owner == null)
48+
if (owner == null) {
4649
return result;
50+
}
4751

4852
processedExpressions.Add(this, result);
4953
Owner.Remap(offset, processedExpressions);

Orm/Xtensive.Orm/Orm/Linq/Expressions/KeyExpression.cs

Lines changed: 73 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,10 @@
44
// Created by: Alexis Kochetov
55
// Created: 2009.05.05
66

7-
using System;
87
using System.Collections.Generic;
98
using System.Linq;
109
using System.Linq.Expressions;
1110
using System.Reflection;
12-
using Xtensive.Collections;
1311
using Xtensive.Core;
1412
using Xtensive.Orm.Model;
1513
using TypeInfo = Xtensive.Orm.Model.TypeInfo;
@@ -18,23 +16,29 @@ namespace Xtensive.Orm.Linq.Expressions
1816
{
1917
internal class KeyExpression : PersistentFieldExpression
2018
{
21-
public TypeInfo EntityType { get; private set; }
22-
public System.Collections.ObjectModel.ReadOnlyCollection<FieldExpression> KeyFields { get; private set; }
19+
public TypeInfo EntityType { get; }
20+
public IReadOnlyList<FieldExpression> KeyFields { get; }
2321

2422
public override Expression Remap(int offset, Dictionary<Expression, Expression> processedExpressions)
2523
{
26-
if (!CanRemap)
24+
if (!CanRemap) {
2725
return this;
26+
}
2827

29-
Expression value;
30-
if (processedExpressions.TryGetValue(this, out value))
31-
return value;
28+
return processedExpressions.TryGetValue(this, out var value)
29+
? value
30+
: RemapWithNoCheck(offset, processedExpressions);
31+
}
3232

33+
// Having this code as a separate method helps to avoid closure allocation during Remap call
34+
// in case processedExpressions dictionary already contains a result.
35+
private Expression RemapWithNoCheck(int offset, Dictionary<Expression, Expression> processedExpressions)
36+
{
3337
var mapping = new Segment<int>(Mapping.Offset + offset, Mapping.Length);
34-
var fields = KeyFields
35-
.Select(f => (FieldExpression)f.Remap(offset, processedExpressions))
36-
.ToList()
37-
.AsReadOnly();
38+
39+
FieldExpression Remap(FieldExpression f) => (FieldExpression) f.Remap(offset, processedExpressions);
40+
41+
var fields = KeyFields.Select(Remap).ToArray(KeyFields.Count);
3842
var result = new KeyExpression(EntityType, fields, mapping, UnderlyingProperty, OuterParameter, DefaultIfEmpty);
3943

4044
processedExpressions.Add(this, result);
@@ -43,46 +47,55 @@ public override Expression Remap(int offset, Dictionary<Expression, Expression>
4347

4448
public override Expression Remap(int[] map, Dictionary<Expression, Expression> processedExpressions)
4549
{
46-
if (!CanRemap)
50+
if (!CanRemap) {
4751
return this;
52+
}
4853

49-
Expression value;
50-
if (processedExpressions.TryGetValue(this, out value))
54+
if (processedExpressions.TryGetValue(this, out var value)) {
5155
return value;
56+
}
5257

5358
var segment = new Segment<int>(map.IndexOf(Mapping.Offset), Mapping.Length);
54-
System.Collections.ObjectModel.ReadOnlyCollection<FieldExpression> fields;
59+
var fields = new FieldExpression[KeyFields.Count];
5560
using (new SkipOwnerCheckScope()) {
56-
fields = KeyFields
57-
.Select(f => f.Remap(map, processedExpressions))
58-
.Where(f => f != null)
59-
.Cast<FieldExpression>()
60-
.ToList()
61-
.AsReadOnly();
62-
}
63-
if (fields.Count != KeyFields.Count) {
64-
if (SkipOwnerCheckScope.IsActive) {
65-
processedExpressions.Add(this, null);
66-
return null;
61+
for (var index = 0; index < fields.Length; index++) {
62+
var field = (FieldExpression)KeyFields[index].Remap(map, processedExpressions);
63+
if (field == null) {
64+
if (SkipOwnerCheckScope.IsActive) {
65+
processedExpressions.Add(this, null);
66+
return null;
67+
}
68+
throw Exceptions.InternalError(Strings.ExUnableToRemapKeyExpression, OrmLog.Instance);
69+
}
70+
71+
fields[index] = field;
6772
}
68-
throw Exceptions.InternalError(Strings.ExUnableToRemapKeyExpression, OrmLog.Instance);
6973
}
7074
var result = new KeyExpression(EntityType, fields, segment, UnderlyingProperty, OuterParameter, DefaultIfEmpty);
7175

7276
processedExpressions.Add(this, result);
7377
return result;
7478
}
7579

76-
public override Expression BindParameter(ParameterExpression parameter, Dictionary<Expression, Expression> processedExpressions)
80+
public override Expression BindParameter(
81+
ParameterExpression parameter, Dictionary<Expression, Expression> processedExpressions)
7782
{
78-
Expression value;
79-
if (processedExpressions.TryGetValue(this, out value))
83+
if (processedExpressions.TryGetValue(this, out var value)) {
8084
return value;
85+
}
8186

82-
var fields = KeyFields
83-
.Select(f => (FieldExpression)f.BindParameter(parameter, processedExpressions))
84-
.ToList()
85-
.AsReadOnly();
87+
return BindParameterWithNoCheck(parameter, processedExpressions);
88+
}
89+
90+
// Having this code as a separate method helps to avoid closure allocation during BindParameter call
91+
// in case processedExpressions dictionary already contains a result.
92+
private Expression BindParameterWithNoCheck(
93+
ParameterExpression parameter, Dictionary<Expression, Expression> processedExpressions)
94+
{
95+
FieldExpression BindParameter(FieldExpression f)
96+
=> (FieldExpression) f.BindParameter(parameter, processedExpressions);
97+
98+
var fields = KeyFields.Select(BindParameter).ToArray(KeyFields.Count);
8699
var result = new KeyExpression(EntityType, fields, Mapping, UnderlyingProperty, parameter, DefaultIfEmpty);
87100

88101
processedExpressions.Add(this, result);
@@ -91,14 +104,21 @@ public override Expression BindParameter(ParameterExpression parameter, Dictiona
91104

92105
public override Expression RemoveOuterParameter(Dictionary<Expression, Expression> processedExpressions)
93106
{
94-
Expression value;
95-
if (processedExpressions.TryGetValue(this, out value))
107+
if (processedExpressions.TryGetValue(this, out var value)) {
96108
return value;
109+
}
110+
111+
return RemoveOuterParameterWithNoCheck(processedExpressions);
112+
}
113+
114+
// Having this code as a separate method helps to avoid closure allocation during RemoveOuterParameter call
115+
// in case processedExpressions dictionary already contains a result.
116+
private Expression RemoveOuterParameterWithNoCheck(Dictionary<Expression, Expression> processedExpressions)
117+
{
118+
FieldExpression RemoveOuterParameter(FieldExpression f)
119+
=> (FieldExpression) f.RemoveOuterParameter(processedExpressions);
97120

98-
var fields = KeyFields
99-
.Select(f => (FieldExpression)f.RemoveOuterParameter(processedExpressions))
100-
.ToList()
101-
.AsReadOnly();
121+
var fields = KeyFields.Select(RemoveOuterParameter).ToArray(KeyFields.Count);
102122
var result = new KeyExpression(EntityType, fields, Mapping, UnderlyingProperty, null, DefaultIfEmpty);
103123

104124
processedExpressions.Add(this, result);
@@ -108,21 +128,25 @@ public override Expression RemoveOuterParameter(Dictionary<Expression, Expressio
108128
public static KeyExpression Create(TypeInfo entityType, int offset)
109129
{
110130
var mapping = new Segment<int>(offset, entityType.Key.TupleDescriptor.Count);
111-
var fields = entityType.Columns
112-
.Where(c => c.IsPrimaryKey)
113-
.OrderBy(c => c.Field.MappingInfo.Offset)
114-
.Select(c => FieldExpression.CreateField(c.Field, offset))
115-
.ToList()
116-
.AsReadOnly();
117-
return new KeyExpression(entityType, fields, mapping,WellKnownMembers.IEntityKey, null, false);
131+
132+
FieldExpression CreateField(ColumnInfo c) => FieldExpression.CreateField(c.Field, offset);
133+
134+
var fields = entityType.IsLocked
135+
? entityType.Key.Columns.Select(CreateField).ToArray(entityType.Key.Columns.Count)
136+
: entityType.Columns
137+
.Where(c => c.IsPrimaryKey)
138+
.OrderBy(c => c.Field.MappingInfo.Offset)
139+
.Select(CreateField)
140+
.ToArray();
141+
return new KeyExpression(entityType, fields, mapping, WellKnownMembers.IEntityKey, null, false);
118142
}
119143

120144

121145
// Constructors
122146

123147
private KeyExpression(
124148
TypeInfo entityType,
125-
System.Collections.ObjectModel.ReadOnlyCollection<FieldExpression> keyFields,
149+
IReadOnlyList<FieldExpression> keyFields,
126150
Segment<int> segment,
127151
PropertyInfo underlyingProperty,
128152
ParameterExpression parameterExpression,

0 commit comments

Comments
 (0)