Author Topic: ObjectId.GetObject()  (Read 9326 times)

0 Members and 1 Guest are viewing this topic.

TheMaster

  • Guest
ObjectId.GetObject()
« on: July 06, 2012, 12:39:49 PM »
Just recently, I happened to be peeking at the source code for ObjectId, and do not remember having ever seen this before, but as soon as I did, I realized that this method is terribly inefficient and should not be called in a loop that runs a huge number of times.

Here is the source code for ObjectId.GetObject() as rendered by ILSpy:

Code: [Select]

public DBObject GetObject(OpenMode mode, bool openErased, bool forceOpenOnLockedLayer)
{
  Database database;
  if (ref this != null)
  {
    IntPtr unmanagedPointer = new IntPtr(<Module>.AcDbObjectId.database(ref this));
    database = Database.Create(unmanagedPointer, false);
  }
  else
  {
    database = null;
  }
  return database.TransactionManager.TopTransaction.GetObject(
            this, mode, openErased, forceOpenOnLockedLayer);
}


If you look carefully, you'll notice that every single call to this method results in the creation of a managed wrapper for the Database that contains the ObjectId, and then a call to it's TransactionManager.TopTransaction properties, to get the top transaction, whose GetObject() method is then called.

So, I will be revising my code to remove calls to ObjectId.GetObject() in cases where it is being used in a loop that can execute a potentially huge number of times, and replacing it with a call to the GetObject() method of the TransactionManager for the database I'm operating on.

If performance is critical, I would suggest you consider doing the same.

« Last Edit: July 06, 2012, 12:47:58 PM by TheMaster »

Jeff H

  • Needs a day job
  • Posts: 6150
Re: ObjectId.GetObject()
« Reply #1 on: July 06, 2012, 01:30:34 PM »
Thanks for the heads up!
 
No error checking or checking if database is null, but
 
Would you do something along the lines of
 
Code: [Select]

         public static DBObject OpenObject(this ObjectId id, OpenMode mode = OpenMode.ForRead, bool openErased = false, bool openObjectOnLockedLayer = false)
        {
            return id.Database.TransactionManager.TopTransaction.GetObject(id, mode, openErased, openObjectOnLockedLayer);
        }

TheMaster

  • Guest
Re: ObjectId.GetObject()
« Reply #2 on: July 06, 2012, 01:47:10 PM »
Thanks for the heads up!
 
No error checking or checking if database is null, but
 
Would you do something along the lines of
 
Code: [Select]

         public static DBObject OpenObject(this ObjectId id, OpenMode mode = OpenMode.ForRead, bool openErased = false, bool openObjectOnLockedLayer = false)
        {
            return id.Database.TransactionManager.TopTransaction.GetObject(id, mode, openErased, openObjectOnLockedLayer);
        }



No, I wouldn't do that.  There's no problem with ObjectId.GetObject() for casual use. It's when it's being called a huge number of times in a loop that it can have a significant effect on performance.

The only cases where I'm replacing ObjectId.GetObject() are in loops where it's called in the loop body, and the loop can execute a huge number of times. I'll revise the code to get the TransactionManager from the database before entering the loop and assign it to a variable that I'll use inside the loop to open objects.

Something like this (abridged):

Before:

Code: [Select]
public static IEnumerable<T> GetObjects<T>( this BlockTableRecord btr ) where T: Entity
{
  RXClass rxclass = RXClass.GetClass( typeof( T ) );
  foreach( ObjectId id in btr )
  {
    if( id.ObjectClass.IsDerivedFrom( rxclass ) )
      yield return (T) id.GetObject( OpenMode.ForRead, false, false );
  }
}

After:

Code: [Select]
public static IEnumerable<T> GetObjects<T>( this BlockTableRecord btr ) where T: Entity
{
  TransactionManager tm = btr.Database.TransactionManager;
  RXClass rxclass = RXClass.GetClass( typeof( T ) );
  foreach( ObjectId id in btr )
  {
    if( id.ObjectClass.IsDerivedFrom( rxclass ) )
      yield return (T) tm.GetObject( id, OpenMode.ForRead, false, false );
  }
}

That simple revision effectively eliminates all of the code in the body of ObjectId.GetObject().

And for those who seem to prefer to marginalize optimizations like this one,
this might help them understand why high-frequency use of ObjectId.GetObject().
is a waste of clock cycles.

This is the functional-equivalent of what is happening when ObjectId.GetObject()
is used in the above GetObjects<T>() "before" example, with the code from the
body of ObjectId.GetObject() inlined:

Code: [Select]
public static IEnumerable<T> GetObjects<T>( this BlockTableRecord btr ) where T: Entity
{
  RXClass rxclass = RXClass.GetClass( typeof( T ) );
  foreach( ObjectId id in btr )
  {
    if( id.ObjectClass.IsDerivedFrom( rxclass ) )
      yield return (T) id.Database.TransactionManager.TopTransaction.GetObject(
          OpenMode.ForRead, false, false );
  }
}

Fixing that requires very little effort (wasted or not), but recognizing it requires
a little skill and experience.
« Last Edit: July 07, 2012, 02:33:33 PM by TheMaster »

Jeff H

  • Needs a day job
  • Posts: 6150
Re: ObjectId.GetObject()
« Reply #3 on: July 07, 2012, 01:24:03 PM »
Oops!
 
If I am understanding correctly the example I posted does absolutely nothing to help, or if anything would make it worse.
Using ILspy for ObjectId.Database property
Code - C#: [Select]
  1. // Autodesk.AutoCAD.DatabaseServices.ObjectId
  2. public unsafe Database Database
  3. {
  4.  get
  5.  {
  6.   if (ref this != null)
  7.   {
  8.    IntPtr unmanagedPointer = new IntPtr(<Module>.AcDbObjectId.database(&this));
  9.    return Database.Create(unmanagedPointer, false);
  10.   }
  11.   return null;
  12.  }
  13. }
  14.  

Then on top of that for the Database.TransactionManager
 
Code - C#: [Select]
  1.  
  2.  // Autodesk.AutoCAD.DatabaseServices.Database
  3. public TransactionManager TransactionManager
  4. {
  5.  get
  6.  {
  7.   IntPtr unmanagedPointer = new IntPtr(<Module>.AcDbDatabase.transactionManager(this.GetImpObj()));
  8.   return (TransactionManager)RXObject.Create(unmanagedPointer, false);
  9.  }
  10. }
  11.  

Then TopTransaction
Code - C#: [Select]
  1. // Autodesk.AutoCAD.DatabaseServices.TransactionManager
  2. public unsafe virtual Transaction TopTransaction
  3. {
  4.  get
  5.  {
  6.   AcDbTransactionManager* expr_06 = this.GetImpObj();
  7.   AcTransaction* ptr = calli(AcTransaction* modopt(System.Runtime.CompilerServices.CallConvCdecl)(System.IntPtr), expr_06, *(*(long*)expr_06   88L));
  8.   if (ptr != null)
  9.   {
  10.    IntPtr unmanagedPointer = new IntPtr((void*)ptr);
  11.    return new Transaction(unmanagedPointer, false);
  12.   }
  13.   return null;
  14.  }
  15. }
  16.  

Then calling GetObject from the Transaction object is the same the thing as TransactionManager.GetObject with the exception of calling CheckTopTransaction first
Code - C#: [Select]
  1. // Autodesk.AutoCAD.DatabaseServices.Transaction
  2. public virtual DBObject GetObject(ObjectId id, OpenMode mode, [MarshalAs(UnmanagedType.U1)] bool openErased, [MarshalAs(UnmanagedType.U1)] bool forceOpenOnLockedLayer)
  3. {
  4.  this.CheckTopTransaction();
  5.  return TransactionManager.GetObjectInternal(<Module>.AcDbImpTransaction.transactionManager(this.GetImpObj()), id, mode, openErased, forceOpenOnLockedLayer);
  6. }
  7.  
  8.  

Code - C#: [Select]
  1. // Autodesk.AutoCAD.DatabaseServices.TransactionManager
  2. public virtual DBObject GetObject(ObjectId id, OpenMode mode, [MarshalAs(UnmanagedType.U1)] bool openErased, [MarshalAs(UnmanagedType.U1)] bool forceOpenOnLockedLayer)
  3. {
  4.  return TransactionManager.GetObjectInternal(this.GetImpObj(), id, mode, openErased, forceOpenOnLockedLayer);
  5. }
  6.  
  7.  

So if I am understanding this correctly,
Iterating over 5,000 objects the example I posted vs yours, my example would create 14,997 extra unneeded objects(Database, TransactionManager, & TopTransaction---I will give myself one of each) and 5,000 calls CheckTopTransaction.
 
 
Thanks again
 
 
 

TheMaster

  • Guest
Re: ObjectId.GetObject()
« Reply #4 on: July 07, 2012, 02:29:54 PM »
Oops!
 
If I am understanding correctly the example I posted does absolutely nothing to help, or if anything would make it worse.
Using ILspy for ObjectId.Database property
Code - C#: [Select]
  1. // Autodesk.AutoCAD.DatabaseServices.ObjectId
  2. public unsafe Database Database
  3. {
  4.  get
  5.  {
  6.   if (ref this != null)
  7.   {
  8.    IntPtr unmanagedPointer = new IntPtr(<Module>.AcDbObjectId.database(&this));
  9.    return Database.Create(unmanagedPointer, false);
  10.   }
  11.   return null;
  12.  }
  13. }
  14.  

Then on top of that for the Database.TransactionManager
 
Code - C#: [Select]
  1.  
  2.  // Autodesk.AutoCAD.DatabaseServices.Database
  3. public TransactionManager TransactionManager
  4. {
  5.  get
  6.  {
  7.   IntPtr unmanagedPointer = new IntPtr(<Module>.AcDbDatabase.transactionManager(this.GetImpObj()));
  8.   return (TransactionManager)RXObject.Create(unmanagedPointer, false);
  9.  }
  10. }
  11.  

Then TopTransaction
Code - C#: [Select]
  1. // Autodesk.AutoCAD.DatabaseServices.TransactionManager
  2. public unsafe virtual Transaction TopTransaction
  3. {
  4.  get
  5.  {
  6.   AcDbTransactionManager* expr_06 = this.GetImpObj();
  7.   AcTransaction* ptr = calli(AcTransaction* modopt(System.Runtime.CompilerServices.CallConvCdecl)(System.IntPtr), expr_06, *(*(long*)expr_06   88L));
  8.   if (ptr != null)
  9.   {
  10.    IntPtr unmanagedPointer = new IntPtr((void*)ptr);
  11.    return new Transaction(unmanagedPointer, false);
  12.   }
  13.   return null;
  14.  }
  15. }
  16.  

Then calling GetObject from the Transaction object is the same the thing as TransactionManager.GetObject with the exception of calling CheckTopTransaction first
Code - C#: [Select]
  1. // Autodesk.AutoCAD.DatabaseServices.Transaction
  2. public virtual DBObject GetObject(ObjectId id, OpenMode mode, [MarshalAs(UnmanagedType.U1)] bool openErased, [MarshalAs(UnmanagedType.U1)] bool forceOpenOnLockedLayer)
  3. {
  4.  this.CheckTopTransaction();
  5.  return TransactionManager.GetObjectInternal(<Module>.AcDbImpTransaction.transactionManager(this.GetImpObj()), id, mode, openErased, forceOpenOnLockedLayer);
  6. }
  7.  
  8.  

Code - C#: [Select]
  1. // Autodesk.AutoCAD.DatabaseServices.TransactionManager
  2. public virtual DBObject GetObject(ObjectId id, OpenMode mode, [MarshalAs(UnmanagedType.U1)] bool openErased, [MarshalAs(UnmanagedType.U1)] bool forceOpenOnLockedLayer)
  3. {
  4.  return TransactionManager.GetObjectInternal(this.GetImpObj(), id, mode, openErased, forceOpenOnLockedLayer);
  5. }
  6.  
  7.  

So if I am understanding this correctly,
Iterating over 5,000 objects the example I posted vs yours, my example would create 14,997 extra unneeded objects(Database, TransactionManager, & TopTransaction---I will give myself one of each) and 5,000 calls CheckTopTransaction.
 
 
Thanks again

The last snippet from my post shows what is actually happening when ObjectId.GetObject() is called in the foreach() block (by simply replacing the call to that method, with the code from its body) 

The overhead of creating all the additional managed wrappers can be significant, but because AutoCAD is single-threaded and the garbage collector runs in a high-priority background thread, on a multi-core system, the GC will run on another under-utilized CPU core, so the overhead of garbage-collecting the managed wrappers and managing and calling their finalizers is actually not much of an issue.

If you want to see how much additional overhead is involved, edit acad.exe.config to disable asynchronous garbage collection, and you should see a significant and perceptible degradation of performance.

TheMaster

  • Guest
Re: ObjectId.GetObject()
« Reply #5 on: July 13, 2012, 03:46:40 PM »
While searching for something unrelated, I came across this thread, which seems to provide some performance insight into how relatively-inefficient ObjectId.GetObject() can be, when called from the body of a loop (or from a function passed to a LINQ method that operates on a sequence of ObjectIds).

The code below shows some of the revisions I've made to some of my own GetObjects<T>() implementations to avoid calls to ObjectId.GetObject().

Also note that while I've not tested it, TransactionManager.GetObject() should be slightly faster than Transaction.GetObject() because both delegate to TransactionManager.GetObjectInternal(), and the Transaction does not cache a reference to the TransactionManager (which it probably should do) and hence, it must get a managed wrapper for the TransactionManager on each call.

Code - C#: [Select]
  1.  
  2.  
  3.  
  4. public static class AcDbExtensionMethods
  5. {
  6.  
  7.   // This is a modified verison of the well-known GetObjects<T>() LINQ
  8.   // extension methods that have appeared in various threads here, by
  9.   // multiple authors.
  10.   // This code also includes other optimizations related to comparing
  11.   // the RXClasses of objectids for equivalence (this optimization has
  12.   // been prototyped and discussed on other threads here).
  13.   //
  14.   // The optimization presented here shows how to avoid calling
  15.   // ObjectId.GetObject() from a method that receives only a sequence
  16.   // of ObjectIds, without any database-resident container. To get
  17.   // the TransactionManager, we use the first ObjectId in the sequence.
  18.   // Note that this method presumes that all elements in the source
  19.   // sequence are from the same database, and makes no attempt to check
  20.   // for that.
  21.  
  22.  
  23.   public static IEnumerable<T> GetObjects<T>(
  24.     this IEnumerable<ObjectId> ids,
  25.     OpenMode mode = OpenMode.ForRead,
  26.     bool openErased = false,
  27.     bool forceOpenOnLockedLayers = false ) where T: DBObject
  28.   {
  29.     // If the source sequence is empty, do nothing
  30.     if( ids.Any() )
  31.     {
  32.       // Get the transaction manager from the first element in the sequence:  
  33.       TransactionManager tm = ids.First().Database.TransactionManager;
  34.  
  35.       // The rest is typical, with the exception that we call the
  36.       // TransactionManager's GetObject() method rather than the
  37.       // ObjectId's GetObject() method:
  38.  
  39.       RXClass rxclass = RXClass.GetClass( typeof( T ) );
  40.       if( typeof( T ) == typeof( DBObject ) )
  41.       {
  42.         foreach( ObjectId id in ids )
  43.         {
  44.           yield return (T) tm.GetObject( id, mode, openErased, forceOpenOnLockedLayers );  
  45.         }
  46.       }
  47.      
  48.       // If at least one RXClass is derived from the runtime
  49.       // class for the generic argument type, then we need to
  50.       // call IsDerivedFrom() and the managed-to-native thunk.
  51.       // Otherwise, we can avoid calling IsDerivedFrom() and
  52.       // instead just compare each ObjectId's ObjectClass to
  53.       // the target RXClass:
  54.        
  55.       else if( rxclass.GetHasChildren() )
  56.       {
  57.         foreach( ObjectId id in ids )
  58.         {
  59.           if( id.ObjectClass.IsDerivedFrom( rxclass ) )
  60.             yield return (T) tm.GetObject( id, mode, openErased, forceOpenOnLockedLayers );  
  61.         }
  62.       }
  63.       else  
  64.       {
  65.         foreach( ObjectId id in ids )
  66.         {
  67.           if( id.ObjectClass == rxclass )
  68.             yield return (T) tm.GetObject( id, mode, openErased, forceOpenOnLockedLayers );  
  69.         }
  70.       }
  71.     }
  72.   }
  73.  
  74.  
  75.   // Returns true if at least one RXClass is derived from
  76.   // the given RXClass:
  77.  
  78.   public static bool GetHasChildren( this RXClass rxclass )
  79.   {
  80.     foreach( DictionaryEntry e in SystemObjects.ClassDictionary )
  81.     {
  82.       if( rxclass == ( (RXClass) e.Value ).MyParent )
  83.         return true;
  84.     }
  85.     return false;
  86.   }
  87.  
  88. }
  89.  
  90.  
  91.  
« Last Edit: May 15, 2013, 12:04:41 AM by TT »

It's Alive!

  • Retired
  • Needs a day job
  • Posts: 8704
  • AKA Daniel
Re: ObjectId.GetObject()
« Reply #6 on: July 13, 2012, 09:37:17 PM »
Wow that is pretty heavy lifting, in Bricscad it just opens the object, of course ODA's system for managing underlying unmanaged objects is quite different... and even adds a bit more thread safety  :-)

CADbloke

  • Bull Frog
  • Posts: 342
  • Crash Test Dummy
Re: ObjectId.GetObject()
« Reply #7 on: May 14, 2013, 04:07:10 PM »
Code - C#: [Select]
  1.           yield return tm.GetObject( id, mode, openErased, forceOpenOnLockedLayers );  

For those playing at home (just me?) Line 44 of the above needs to read...
Code - C#: [Select]
  1.           yield return (T) tm.GetObject( id, mode, openErased, forceOpenOnLockedLayers );  
Note the cast to (T).

TheMaster

  • Guest
Re: ObjectId.GetObject()
« Reply #8 on: May 15, 2013, 12:02:57 AM »
Code - C#: [Select]
  1.           yield return tm.GetObject( id, mode, openErased, forceOpenOnLockedLayers );  

For those playing at home (just me?) Line 44 of the above needs to read...
Code - C#: [Select]
  1.           yield return (T) tm.GetObject( id, mode, openErased, forceOpenOnLockedLayers );  
Note the cast to (T).

Thanks for pointing that out.  This is one of several bugs that resulted from removing dependencies on other code.

FWIW, that version has been depreciated, after testing revealing that the DisposableWrapper's == operator has about the same overhead as a call to RXClass.IsDerivedFrom(), so if you've been following other discussions where that was mentioned, you can eliminate the clause that calls GetHasChildren() and just use the loop that calls IsDerivedFrom() because there is almost no perceptible difference between then, even with hundreds of thousands of objects.
« Last Edit: May 15, 2013, 12:09:27 AM by TT »

nekitip

  • Guest
Re: ObjectId.GetObject()
« Reply #9 on: May 17, 2013, 04:10:31 AM »
there was a blog a year ago: ObjectId.GetObject vs. Transaction.GetObject
http://spiderinnet1.typepad.com/blog/2012/05/objectidgetobject-vs-transactiongetobject-in-autocad-net.html

I never really tested it cause I was scared that it will complicate my code far more than the speed gain was. ...but... maybe it will help you to know.