Optimizations of Domain build (#83)

[SergeiPavlov]

• Optimize TypeInfo.Validators

• Optimization: create FieldInfo.Assocations on demand

• Avoid unnecessary .ToList() in TopologicalSorter.Sort()

• DO_SAFE_COLLECTION_WRAPPER constant

• readonly struct ColumnIndexMap

• Optimize IReadOnlyList in SelectProvider

[alex-kulakov]

I suggest removing all these new members and do following in FieldInfo ctor:

private FieldInfo(TypeInfo declaringType, TypeInfo reflectedType, FieldAttributes attributes)
    {
      Attributes = attributes;
      this.declaringType = declaringType;
      this.reflectedType = reflectedType;
      Fields = IsEntity || IsStructure 
        ? new FieldInfoCollection(this, "Fields") 
        : FieldInfoCollection.Empty;

      associations = (IsEntity || IsEntitySet) ? new NodeCollection<AssociationInfo>(this, "Associations") : NodeCollection<AssociationInfo>.Empty;
    }

This makes only the fields which suppose to have association have them, for the rest of the fields (of primitive types or persistent structure types) it will be reference to single object which is locked and can't be changes.

[alex-kulakov]

I did tests on Xtensive.Orm.Tests.Upgrade.HugeModelUpgrade.RegularModel types set
The change in previous comment helped to reduce number of NodeCollection<AssociationInfo> instances from 8613 to 4269.

Next change helped to squeeze some more water out of stone and reach 2135 instances. This will vary because of model but still will reduce count of instances.

So, the change...
I suggest to improve AssociationInfo.Ancestors collection by doing following...
First, declare the property like so

    public NodeCollection<AssociationInfo> Ancestors
    {
      get { return ancestors ??= new NodeCollection<AssociationInfo>(this, "Ancestors"); }
    }

and remove property initialization from AssociationInfo ctor.

Second, override Lock within the class like so

    public override void Lock(bool recursive)
    {
      base.Lock(recursive);
      if (!recursive)
        return;
      if (ancestors is null || ancestors.Count == 0)
        ancestors = NodeCollection<AssociationInfo>.Empty;
      else
        ancestors.Lock(false);
    }

which shrinks final model. Both cases in IF are possible.

Going further, we can improve all mentions like association.Ancestors.AddRange(...) with check for count before accessing Ancestors property:

    if (field.Associations.Count > 0)
      association.Ancestors.AddRange(field.Associations);

which also can affect some models. The model which I used have no interfaces so it is not affected by this but among all tests there are scenarios when association.Ancestors.AddRange(...) is called for empty item source collection.

[SergeiPavlov]

Done all these changes

[alex-kulakov]

No, not all these changes applied otherwise there wouldn't be
public IReadOnlyList<AssociationInfo> Associations => associations; is still exists. I suggested to revert the region I mentioned above completely and so API would stay untouched.

[SergeiPavlov]

Got it

Reverted these methods

[alex-kulakov]

Maybe it was my fault and I was unclear. Let me explain what I suggested clearer. FieldInfo.Associations should be

public NodeCollection<AssociationInfo> Associations  => associations;

Methods

• FieldInfo.ContainsAssociation(string),
• FieldInfo.AddAssociation(AssociationInfo),
• FieldInfo.AddAssociations(IReadOnlyList<AssociationInfo>),
• FieldInfo.RemoveAssociation(AssociationInfo)

should be removed and usages of the methods should be reverted to usage of NodeCollection<AssociationInfo> API.

I see advantage of having IReadOnlyList<AssociationInfo> - it makes it seem read-only. But there are some disadvantages - it is in use during Domain life quite a lot, and every time the collection should be casted to interface before return which would hurt performance and I see you prefer not to allow such casts; also interface doesn't make it read-only and anyone can see what type is underneath and nothing prevents from casting it to NodeCollection<AssociationInfo> and call methods, so only locking makes it read-only and it happens anyway.

So, we don't need to return NodeCollection<AssociationInfo> as interface. It makes working with this collection awkward and we would be forced to bloat API with useless proxy methods or properties to perform operations on it. Also, we would probably break some user code.

[SergeiPavlov]

I agree and as I said the suggested changes were made.
And I simplified the property to:
public NodeCollection<AssociationInfo> Associations { get; private set; }

No need for separate field associations