5

Closed

Performance issues in IfcSpatialStructureExtensions and IfcPersistedInstanceCache

description

When adding a large amount of objects to a spatial structure there are a number of performance issues. First a rather simple one. The method IFcSpatialStructureExtensions.AddElement uses a very inefficient
Count() == 0
Which takes O(N) time when a call to Any() would be O(1). Since the same method later uses "First()" the two calls can actually be grouped into a single FirstOrDefault(), suggestion:
IEnumerable<IfcRelContainedInSpatialStructure> relatedElements = se.ContainsElements;
         var first = relatedElements.FirstOrDefault();
         if (first == null) //none defined create the relationship
         {
            IfcRelContainedInSpatialStructure relSe = se.ModelOf.Instances.New<IfcRelContainedInSpatialStructure>();
            relSe.RelatingStructure = se;
            relSe.RelatedElements.Add_Reversible(prod);
         }
         else
         {
            first.RelatedElements.Add_Reversible(prod);
         }
I thouhgt my performance issues related to this method only, but after using this code instead I found that even calling First() will cause a massive speed penalty on the relatedElements collection. The reason is that down inside IfcPersistedInstanceCache there are calls to ToList() for the collections being enumerated (to avoid concurrency issues, the comment says). It appears that even to check the count of objects, this will clone entire dictionaries into lists. This means that checking the count each time in a loop like you do when adding multiple objects, will now be an O(N^2) operation!

The ToList() calls will probably need to be removed from the cache class, something that should be possible if the cache dictionaries are replaced with ConcurrentDictionary<K,V>.
Closed Oct 27, 2016 at 12:59 PM by martin1cerny
This project has been moved to GitHub. Please, raise an issue there if it is still a problem in the latest xBIM4.

comments

andyward4p wrote Aug 9, 2013 at 2:57 PM

Good spot. We need to take a look at this.

wrote Sep 22, 2013 at 1:03 AM

andyward4p wrote Oct 18, 2013 at 8:41 AM

Laurence can you take a look?

wrote Oct 27, 2016 at 12:59 PM