Skip to content

ARCH-6354: Optimize Get/SetFieldValue() to use integer indexes instead of search in Dictionary by string field name #158

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 31 commits into from
Dec 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
352a90e
ARCH-6354: Optimize Get/SetFieldValue() to use integer indexes instea…
SergeiPavlov Nov 13, 2023
2f786ef
Correct for IssueJira0538 test case
SergeiPavlov Nov 14, 2023
b8e376f
Refactor
SergeiPavlov Nov 14, 2023
38d5306
Refactor
SergeiPavlov Nov 14, 2023
fc90782
non-Public fields too
SergeiPavlov Nov 14, 2023
0d00544
add comment
SergeiPavlov Nov 14, 2023
6f7044b
rename
SergeiPavlov Nov 14, 2023
fd62196
Merge remote-tracking branch 'origin/master-servicetitan' into indexe…
SergeiPavlov Nov 15, 2023
0f73f01
Fix issue witn `new` property specifier
SergeiPavlov Nov 15, 2023
1ba886e
Fix Flaky test
SergeiPavlov Nov 15, 2023
11355a3
Fix BuildPersistentFields()
SergeiPavlov Nov 15, 2023
d9ebad5
Merge branch 'master-servicetitan' into indexedFields
SergeiPavlov Nov 16, 2023
13e73b1
Skip overridden persistent properties
SergeiPavlov Nov 16, 2023
510e7e2
Correctly process overriddent properties
SergeiPavlov Nov 16, 2023
7026d42
DOn't ignore abstract props
SergeiPavlov Nov 16, 2023
9b82e2f
Merge branch 'master-servicetitan' into indexedFields
SergeiPavlov Nov 16, 2023
3d54ef9
Add EOL
SergeiPavlov Nov 16, 2023
f4d63a9
Upgrade to Ubuntu 22.04
SergeiPavlov Nov 17, 2023
e858c65
Merge branch 'master-servicetitan' into indexedFields
SergeiPavlov Nov 17, 2023
64b486f
Merge branch 'master-servicetitan' into indexedFields
SergeiPavlov Nov 17, 2023
988f3e6
Fix assigning prop index
SergeiPavlov Nov 17, 2023
baa55ac
Fix processing virtual fields
SergeiPavlov Nov 17, 2023
0cdcfed
Minor fix
SergeiPavlov Nov 17, 2023
31c86d8
process IsStructure bases
SergeiPavlov Nov 17, 2023
a337341
Fix a few cases
SergeiPavlov Nov 17, 2023
4bfd05c
Fix QueryCompositeTest case
SergeiPavlov Nov 19, 2023
d0c5a43
Process Recycled fields
SergeiPavlov Nov 19, 2023
6284435
Optimize Field mapping
SergeiPavlov Nov 19, 2023
eb03685
Refactor
SergeiPavlov Nov 21, 2023
5c32d0e
Merge branch 'master-servicetitan' into indexedFields
SergeiPavlov Nov 28, 2023
4d51746
Merge branch 'master-servicetitan' into indexedFields
SergeiPavlov Dec 19, 2023
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
2 changes: 0 additions & 2 deletions Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,6 @@

<ItemGroup>
<SourceRoot Include="$(SolutionDir)"/>
<PackageReference Condition="$(DoGeneratePackage) == 'true'"
Include="Microsoft.SourceLink.GitHub" Version="1.0.0" PrivateAssets="All"/>
<None Include="$(DoPackageIcon)" Visible="false" >
<PackagePath>.</PackagePath>
<Pack>true</Pack>
Expand Down
71 changes: 71 additions & 0 deletions Orm/Xtensive.Orm/Orm/Model/TypeInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,14 @@
using System.Collections.ObjectModel;
using System.Diagnostics;
using System.Linq;
using System.Reflection;
using System.Threading;
using JetBrains.Annotations;
using Xtensive.Core;
using Xtensive.Orm.Internals;
using Xtensive.Orm.Validation;
using Xtensive.Orm.Upgrade;
using Xtensive.Reflection;
using Xtensive.Tuples;
using Xtensive.Tuples.Transform;
using Tuple = Xtensive.Tuples.Tuple;
Expand All @@ -42,6 +45,10 @@ public sealed class TypeInfo : SchemaMappedNode
/// </summary>
public const int MinTypeId = 100;

private static readonly Type
TypeEntity = typeof(Entity)
, TypeStructure = typeof(Structure);

private static volatile int CurrentSharedId = 0;
private static readonly ConcurrentDictionary<Type, int> TypeToSharedId = new();

Expand Down Expand Up @@ -421,6 +428,9 @@ public FieldInfoCollection Fields
get { return fields; }
}

private FieldInfo[] persistentFields;
internal FieldInfo[] PersistentFields => persistentFields ??= BuildPersistentFields();

/// <summary>
/// Gets the field map for implemented interfaces.
/// </summary>
Expand Down Expand Up @@ -913,6 +923,67 @@ private IDictionary<Pair<FieldInfo>, FieldInfo> BuildStructureFieldMapping()
return new ReadOnlyDictionary<Pair<FieldInfo>, FieldInfo>(result);
}

private static IEnumerable<FieldInfo> GetBaseFields(Type type, IEnumerable<FieldInfo> fields, bool bRoot)
{
if (type == TypeEntity || type == TypeStructure) {
return Array.Empty<FieldInfo>();
}
var declared = type.GetProperties(BindingFlags.DeclaredOnly | BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)
.ToDictionary(p => p.Name, p => p.MetadataToken);

return GetBaseFields(type.BaseType, fields, bRoot)
.Concat(
fields.Select(p => (p, declared.TryGetValue(p.UnderlyingProperty.Name, out var token) ? token : 0))
.Where(t => bRoot ? t.Item2 != 0 : t.Item1.UnderlyingProperty.MetadataToken == t.Item2)
.OrderBy(t => t.Item2)
.Select(t => t.Item1)
);
}

private static bool IsOverrideOfVirtual(FieldInfo a, FieldInfo p) =>
a.Name == p.Name
&& a.Name != "TypeId"
&& p.IsInherited
&& a.UnderlyingProperty.GetMethod?.IsVirtual == true
&& p.UnderlyingProperty.GetMethod?.IsVirtual == true;

private FieldInfo[] BuildPersistentFields()
{
var propTypeId = IsEntity ? Fields[nameof(Entity.TypeId)] : null;
bool isRoot = Hierarchy?.Root == this;
var potentialFields = Fields.Where(p => !p.IsDynamicallyDefined && p.Parent == null && p != propTypeId).ToArray();
var recycled = UnderlyingType.GetProperties(BindingFlags.DeclaredOnly | BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)
.Where(p => p.GetAttribute<RecycledAttribute>() != null && !potentialFields.Any(pf => pf.UnderlyingProperty == p));

FieldInfo[] baseFields;
FieldInfo[] ancestorFields = Array.Empty<FieldInfo>();
if (Ancestor != null && Ancestor.UnderlyingType != TypeStructure) {
ancestorFields = Ancestor.PersistentFields;
baseFields = ancestorFields.Select(p => p != null && Fields.TryGetValue(p.Name, out var f) ? f : null).ToArray();
}
else {
baseFields = !(IsEntity || IsStructure)
? Array.Empty<FieldInfo>()
: GetBaseFields(UnderlyingType.BaseType, potentialFields, isRoot).ToArray();
}
var baseFieldsSet = baseFields.ToHashSet();
var props = baseFields.Concat(
potentialFields.Where(p => p.UnderlyingProperty.DeclaringType == UnderlyingType
&& (isRoot
|| p.IsExplicit
|| !baseFieldsSet.Contains(p)
|| ancestorFields.Any(a => IsOverrideOfVirtual(a, p))))
.Select(p => (p, p.UnderlyingProperty.MetadataToken))
.Concat(recycled.Select(p => ((FieldInfo)null, p.MetadataToken)))
.OrderBy(t => t.Item2)
.Select(t => t.Item1)
).Select(p => p?.ReflectedType.IsInterface == true ? FieldMap[p] : p);
if (IsEntity && Ancestor == null) {
props = props.Prepend(propTypeId);
}
return props.ToArray();
}

#endregion

/// <inheritdoc/>
Expand Down
20 changes: 14 additions & 6 deletions Orm/Xtensive.Orm/Orm/Persistent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,9 @@ protected internal object GetFieldValue(string fieldName)
return GetFieldValue(TypeInfo.Fields[fieldName]);
}

protected internal T GetFieldValue<T>(int fieldIndex) =>
GetNormalizedFieldValue<T>(TypeInfo.PersistentFields[fieldIndex]);

/// <summary>
/// Gets the field value.
/// Field value type must be specified precisely.
Expand All @@ -173,10 +176,11 @@ protected internal object GetFieldValue(string fieldName)
/// <typeparam name="T">Field value type.</typeparam>
/// <param name="field">The field.</param>
/// <returns>Field value.</returns>
protected internal T GetFieldValue<T>(FieldInfo field)
protected internal T GetFieldValue<T>(FieldInfo field) =>
GetNormalizedFieldValue<T>(field.ReflectedType.IsInterface ? TypeInfo.FieldMap[field] : field);

protected internal T GetNormalizedFieldValue<T>(FieldInfo field)
{
if (field.ReflectedType.IsInterface)
field = TypeInfo.FieldMap[field];
var fieldAccessor = GetNormalizedFieldAccessor<T>(field);
T result = default(T);
try {
Expand Down Expand Up @@ -358,6 +362,9 @@ protected internal void SetFieldValue(string fieldName, object value)
SetFieldValue(TypeInfo.Fields[fieldName], value);
}

protected internal void SetFieldValue<T>(int fieldIndex, T value) =>
SetNormalizedFieldValue(TypeInfo.PersistentFields[fieldIndex], value, null, null);

/// <summary>
/// Sets the field value.
/// Field value type must be specified precisely.
Expand All @@ -381,10 +388,11 @@ protected internal void SetFieldValue(FieldInfo field, object value)
SetFieldValue(field, value, null, null);
}

internal void SetFieldValue(FieldInfo field, object value, SyncContext syncContext, RemovalContext removalContext)
internal void SetFieldValue(FieldInfo field, object value, SyncContext syncContext, RemovalContext removalContext) =>
SetNormalizedFieldValue(field.ReflectedType.IsInterface ? TypeInfo.FieldMap[field] : field, value, syncContext, removalContext);

internal void SetNormalizedFieldValue(FieldInfo field, object value, SyncContext syncContext, RemovalContext removalContext)
{
if (field.ReflectedType.IsInterface)
field = TypeInfo.FieldMap[field];
SystemSetValueAttempt(field, value);
var fieldAccessor = GetNormalizedFieldAccessor(field);
object oldValue = GetNormalizedFieldValue(field, fieldAccessor);
Expand Down
5 changes: 3 additions & 2 deletions Weaver/Xtensive.Orm.Weaver/Stages/ImportReferencesStage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public override ActionResult Execute(ProcessorContext context)
}
registry.OrmAssembly = ormAssembly;

var intType = context.TargetModule.TypeSystem.Int32;
var stringType = context.TargetModule.TypeSystem.String;
var voidType = context.TargetModule.TypeSystem.Void;

Expand Down Expand Up @@ -59,13 +60,13 @@ public override ActionResult Execute(ProcessorContext context)
var persistentGetter = new MethodReference("GetFieldValue", voidType, persistentType) {HasThis = true};
var getterType = new GenericParameter("!!T", persistentGetter);
persistentGetter.ReturnType = getterType;
persistentGetter.Parameters.Add(new ParameterDefinition(stringType));
persistentGetter.Parameters.Add(new ParameterDefinition(intType));
persistentGetter.GenericParameters.Add(getterType);
registry.PersistentGetterDefinition = context.TargetModule.ImportReference(persistentGetter);

var persistentSetter = new MethodReference("SetFieldValue", voidType, persistentType) {HasThis = true};
var setterType = new GenericParameter("!!T", persistentSetter);
persistentSetter.Parameters.Add(new ParameterDefinition(stringType));
persistentSetter.Parameters.Add(new ParameterDefinition(intType));
persistentSetter.Parameters.Add(new ParameterDefinition(setterType));
persistentSetter.GenericParameters.Add(setterType);
registry.PersistentSetterDefinition = context.TargetModule.ImportReference(persistentSetter);
Expand Down
38 changes: 29 additions & 9 deletions Weaver/Xtensive.Orm.Weaver/Stages/ModifyPersistentTypesStage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
// Created by: Denis Krjuchkov
// Created: 2013.08.21

using System;
using System.Collections.Generic;
using System.Linq;
using Mono.Cecil;
using Xtensive.Orm.Weaver.Tasks;
Expand Down Expand Up @@ -38,8 +40,8 @@ public override ActionResult Execute(ProcessorContext context)
new[] {references.Entity, references.FieldInfo},
};

propertyChecker = (context.Language==SourceLanguage.CSharp)
? (IPersistentPropertyChecker) new CsPropertyChecker()
propertyChecker = (context.Language==SourceLanguage.CSharp)
? (IPersistentPropertyChecker) new CsPropertyChecker()
: new VbPropertyChecker();

foreach (var type in context.PersistentTypes)
Expand Down Expand Up @@ -107,26 +109,44 @@ private void ProcessStructure(ProcessorContext context, TypeInfo type)
context.WeavingTasks.Add(new AddAttributeTask(definition, context.References.StructureTypeAttributeConstructor));
}

private Dictionary<PropertyInfo, int> GetPropertyToIndexMap(TypeInfo type)
{
if (type is null) {
return new();
}
var r = GetPropertyToIndexMap(type.BaseType);
int idx = r.Count == 0 ? 0 : r.Values.Max() + 1;
if (idx == 0 && type.Kind == PersistentTypeKind.Entity) {
idx = 1; // for TypeId
}
foreach (var p in type.Properties.Values.Where(p => p.IsPersistent)
.OrderBy(p => p.Definition.MetadataToken.ToInt32())) {
r[p] = (p.IsOverride && p.BaseProperty.IsPersistent)
? r[p.BaseProperty] // For overridden persistent property assign base property's index
: idx++;
}
return r;
}

private void ProcessFields(ProcessorContext context, TypeInfo type)
{
foreach (var property in type.Properties.Values.Where(p => p.IsPersistent)) {
if (!propertyChecker.ShouldProcess(property, context))
continue;
var typeDefinition = type.Definition;
var propertyToIndex = GetPropertyToIndexMap(type);

var typeDefinition = type.Definition;
foreach (var property in type.Properties.Values.Where(p => p.IsPersistent && propertyChecker.ShouldProcess(p, context))) {
var persistentIndex = propertyToIndex[property];
var propertyDefinition = property.Definition;
var persistentName = property.PersistentName ?? property.Name;
// Backing field
context.WeavingTasks.Add(new RemoveBackingFieldTask(typeDefinition, propertyDefinition));
// Getter
context.WeavingTasks.Add(new ImplementFieldAccessorTask(AccessorKind.Getter,
typeDefinition, propertyDefinition, persistentName));
typeDefinition, propertyDefinition, persistentIndex));
// Setter
if (property.IsKey)
context.WeavingTasks.Add(new ImplementKeySetterTask(typeDefinition, propertyDefinition));
else
context.WeavingTasks.Add(new ImplementFieldAccessorTask(AccessorKind.Setter,
typeDefinition, propertyDefinition, persistentName));
typeDefinition, propertyDefinition, persistentIndex));
if (property.PersistentName!=null)
context.WeavingTasks.Add(new AddAttributeTask(propertyDefinition,
context.References.OverrideFieldNameAttributeConstructor, property.PersistentName));
Expand Down
12 changes: 6 additions & 6 deletions Weaver/Xtensive.Orm.Weaver/Tasks/ImplementFieldAccessorTask.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ internal sealed class ImplementFieldAccessorTask : WeavingTask
{
private readonly TypeDefinition type;
private readonly PropertyDefinition property;
private readonly string persistentName;
private readonly int persistentIndex;
private readonly AccessorKind kind;

public override ActionResult Execute(ProcessorContext context)
Expand All @@ -42,7 +42,7 @@ private void ImplementSetter(ProcessorContext context)
body.Instructions.Clear();
var il = body.GetILProcessor();
il.Emit(OpCodes.Ldarg_0);
il.Emit(OpCodes.Ldstr, persistentName);
il.Emit(OpCodes.Ldc_I4, persistentIndex);
il.Emit(OpCodes.Ldarg_1);
il.Emit(OpCodes.Tail);
il.Emit(OpCodes.Call, accessor);
Expand All @@ -57,7 +57,7 @@ private void ImplementGetter(ProcessorContext context)
body.Instructions.Clear();
var il = body.GetILProcessor();
il.Emit(OpCodes.Ldarg_0);
il.Emit(OpCodes.Ldstr, persistentName);
il.Emit(OpCodes.Ldc_I4, persistentIndex);
il.Emit(OpCodes.Tail);
il.Emit(OpCodes.Call, accessor);
il.Emit(OpCodes.Ret);
Expand All @@ -77,14 +77,14 @@ private MethodReference GetAccessor(ProcessorContext context,
return result;
}

public ImplementFieldAccessorTask(AccessorKind kind, TypeDefinition type, PropertyDefinition property, string persistentName)
public ImplementFieldAccessorTask(AccessorKind kind, TypeDefinition type, PropertyDefinition property, int persistentIndex)
{
ArgumentNullException.ThrowIfNull(type);
ArgumentNullException.ThrowIfNull(property);
this.kind = kind;
this.type = type;
this.property = property;
this.persistentName = persistentName;
this.persistentIndex = persistentIndex;
}
}
}
}