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

SergeiPavlov · web-flow · commit dc6346f97b62 · 2023-12-20T14:49:56.000-08:00
* ARCH-6354: Optimize Get/SetFieldValue() to use integer indexes instead of search by string key

* Correct for IssueJira0538 test case

* Refactor

* Refactor

* non-Public fields too

* add comment

* rename

* Fix issue witn `new` property specifier

* Fix Flaky test

* Fix BuildPersistentFields()

* Skip overridden persistent properties

* Correctly process overriddent properties

* DOn't ignore abstract props

* Add EOL

* Upgrade to Ubuntu 22.04

* Fix assigning prop index

* Fix processing virtual fields

* Minor fix

* process IsStructure bases

* Fix a few cases

* Fix QueryCompositeTest case

* Process Recycled fields

* Optimize Field mapping

* Refactor
diff --git a/Directory.Build.props b/Directory.Build.props
@@ -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>
diff --git a/Orm/Xtensive.Orm/Orm/Model/TypeInfo.cs b/Orm/Xtensive.Orm/Orm/Model/TypeInfo.cs
@@ -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;
@@ -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();
 
@@ -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>
@@ -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/>
diff --git a/Orm/Xtensive.Orm/Orm/Persistent.cs b/Orm/Xtensive.Orm/Orm/Persistent.cs
@@ -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.
@@ -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 {
@@ -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.
@@ -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);
diff --git a/Weaver/Xtensive.Orm.Weaver/Stages/ImportReferencesStage.cs b/Weaver/Xtensive.Orm.Weaver/Stages/ImportReferencesStage.cs
@@ -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;
 
@@ -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);
diff --git a/Weaver/Xtensive.Orm.Weaver/Stages/ModifyPersistentTypesStage.cs b/Weaver/Xtensive.Orm.Weaver/Stages/ModifyPersistentTypesStage.cs
@@ -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;
@@ -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)
@@ -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));
diff --git a/Weaver/Xtensive.Orm.Weaver/Tasks/ImplementFieldAccessorTask.cs b/Weaver/Xtensive.Orm.Weaver/Tasks/ImplementFieldAccessorTask.cs
@@ -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)
@@ -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);
@@ -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);
@@ -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;
     }
   }
-}
+}
