RegisterProperty performance improvement proposal

RegisterProperty performance improvement proposal

Old forum URL: forums.lhotka.net/forums/t/10697.aspx


nelis posted on Tuesday, September 20, 2011

When a business object contains lots of properties (I had 400, I know I should reconcider) RegisterProperty takes relatively much time due to the Sort performed in the PropertyInfoManager.

I adjusted the code to perform the Sort in the IsLocked setter which made the 400 calls to RegisterProperty about 4 times faster.

Why is the list sorted anyway?

Would this change have undesired side-effects?

RockfordLhotka replied on Tuesday, September 20, 2011

After the properties are all registered, they are assigned an index number. The index numbers on the client and server must line up, or serialization will fail.

There's no way to guarantee the order in which .NET 32 bit, .NET 64 bit, SL 2, SL 3, SL 4, SL 5, SL WP7, SL WP7.5, and WinRT initialize the static fields. That is a compiler detail that is specifically indeterminate.

So we needed some way to ensure that the list of registered properties was in the same order in a consistent manner - and sorting the list is a viable solution.

It has been a while, but IsLocked is set to true after CSLA knows that all properties for the type have been registered right? That is probably a good optimization then. My only worry would be if this interferes with inheritance, where the base type also declares properties - that can get complex.

Can you post the code you added to the IsLocked setter?

JonnyBee replied on Wednesday, September 21, 2011

This may also be changed:

from:
        if (list.Count(pi => pi.Name == info.Name) > 0)
          throw new InvalidOperationException(string.Format(Resources.PropertyRegisterDuplicateNotAllowed, info.Name));

to:
        if (list.Any(pi => pi.Name == info.Name))
          throw new InvalidOperationException(string.Format(Resources.PropertyRegisterDuplicateNotAllowed, info.Name));

Any should stop on the first occurrence whereas Count would need to loop through the entire list.

 

 

 

 

nelis replied on Wednesday, September 21, 2011

Jonny,

You are right. Performance-wise LINQ is usually the lesser option. Even Any is probably beaten by a simple foreach and break.

JonnyBee replied on Wednesday, September 21, 2011

I did a small test on my computer on a BO with 4000 properties. Could you try this:

Change following in PropertyInfoManager:

    public static PropertyInfo<T> RegisterProperty<T>(Type objectType, PropertyInfo<T> info)

    {
      var list = GetPropertyListCache(objectType);
      lock (list)
      {
        if (list.IsLocked)
          throw new InvalidOperationException(string.Format(Resources.PropertyRegisterNotAllowed, info.Name, objectType.Name));

        // original code
        //if (list.Any(pi => pi.Name == info.Name))
        //  throw new InvalidOperationException(string.Format(Resources.PropertyRegisterDuplicateNotAllowed, info.Name));

        //list.Add(info);
        //list.Sort();


        var index = list.BinarySearch(info);
        if (index >= 0)
          throw new InvalidOperationException(string.Format(Resources.PropertyRegisterDuplicateNotAllowed, info.Name));

        list.Insert(~index, info);

      }
      return info;
    }

Using BinarySearch, then test for existing (index >= 0) and inserting item in the proper sorted place gave me a perfomance improvement  of  6:1 from original code.

We DO need the check for duplicates and this way the search is a short as possible (in sort order) and items are always inserted in the proper sort place in the list so it fully avoids the need for resorting the list.

JonnyBee replied on Thursday, September 22, 2011

See item  http://www.lhotka.net/cslabugs/edit_bug.aspx?id=960

Changed as per above.

stefan replied on Thursday, September 22, 2011

Can this be backported into 3.8.x?

JonnyBee replied on Thursday, September 22, 2011

Yes, it can.

nelis replied on Wednesday, September 21, 2011

private bool m_IsLocked;

public bool IsLocked {
   get { return m_IsLocked; } 
   set { 
      if (m_IsLocked != value) { 
         m_IsLocked = value; 
         if (m_IsLocked) { 
            lock (this) { 
               Sort(); 
            } 
         } 
      } 
   } 
}

Copyright (c) Marimer LLC