Author Topic: Abort This  (Read 7167 times)

0 Members and 1 Guest are viewing this topic.

TheMaster

  • Guest
Abort This
« on: February 26, 2013, 04:00:23 AM »
It seems that one Autodesk employee that was lecturing at AU this
year wants to be a pot-stirrer, by recommending that managed API
programmers abort transactions when they're being used exclusively
for read-only purposes.

That squarely contradicts what I was told by one of the AutoCAD
architects who also happens to be the creator of the managed API,
which is that transactions should be committed, not aborted, even
when nothing has been changed, because aborting them has major
overhead (see test case below).

Below is the code I use to make comitting transactions implicit.

The code is centered around a class called the ReadOnlyTransaction.

ReadOnlyTransaction is Transaction that cannot be aborted. When a
regular Transaction is disposed, it will implicitly abort, if it
wasn't previously comitted.

When a ReadOnlyTransaction is disposed it will implicitly commit if
not previously comitted. You can use ReadOnlyTransaction in place of
a regular Transaction, when you are not modifying anything, and you
want to be sure the transaction is comitted regardless of anything,
including if an exception stops your code. You can omit the explicit
call to Commit() a ReadOnlyTransaction, because it does that for you
when it's disposed (so it should always be used with using(...), to
ensure that happens). The main benefit of using ReadOnlyTransaction
is that it allows you to exit a using() block at any point, without
having to call Commit() before you do, to ensure the transaction is
comitted. Another case where it can be useful, is where you prompt
for input while a transaction is active, and want to ensure that any
transparent view changes made by the user are not undone, which will
happen if you abort the transaction. If I need to access objects and
prompt for user input afterwards, I can start a ReadOnlyTransaction
for that, and if I later need to modify the database, then I'll start
a regular transaction for that.

The StartReadOnlyTransaction() extension methods of TransactionManager
simplify using the ReadOnlyTransaction. You can simply replace a call
to StartTransaction() with a call to StartReadOnlyTransaction(), and
omit the subsequent call to Commit(), and nothing else needs to change.

I've also posted a simple test case showing why aborting transactions
when there is nothing to roll back, is not a good idea, especially if
it is being done in frequently-called APIs that implicitly start and
end their own transactions internally.

Code - C#: [Select]
  1.  
  2. using System;
  3. using System.Linq;
  4. using Autodesk.AutoCAD.Runtime;
  5. using Autodesk.AutoCAD.Internal;
  6. using AcApTransactionManager = Autodesk.AutoCAD.ApplicationServices.TransactionManager;
  7.  
  8. namespace Autodesk.AutoCAD.DatabaseServices
  9. {
  10.    public class CustomTransaction : Transaction
  11.    {
  12.       public CustomTransaction( Transaction trans )
  13.          : base( trans.UnmanagedObject, trans.AutoDelete )
  14.       {
  15.          Interop.DetachUnmanagedObject( trans );
  16.       }
  17.    }
  18.  
  19.    public class ReadOnlyTransaction : CustomTransaction
  20.    {
  21.       public ReadOnlyTransaction( Transaction trans )
  22.          : base( trans )
  23.       {
  24.       }
  25.  
  26.       public override void Abort()
  27.       {
  28.          base.Commit();
  29.       }
  30.  
  31.       protected override void Dispose( bool disposing )
  32.       {
  33.          if( disposing && base.AutoDelete )
  34.             base.Commit();
  35.          base.Dispose( disposing );
  36.       }
  37.    }
  38.  
  39.    public static class TransactionManagerExtensionMethods
  40.    {
  41.       public static Transaction StartReadOnlyTransaction( this TransactionManager tm )
  42.       {
  43.          if( tm == null )
  44.             throw new ArgumentNullException( "tm" );
  45.          return new ReadOnlyTransaction( tm.StartTransaction() );
  46.       }
  47.  
  48.       public static Transaction StartReadOnlyTransaction( this AcApTransactionManager tm )
  49.       {
  50.          if( tm == null )
  51.             throw new ArgumentNullException( "tm" );
  52.          return new ReadOnlyTransaction( tm.StartTransaction() );
  53.       }
  54.    }
  55.  
  56. }
  57.  
  58.  

Transaction Commit verses Abort test code:

Code - C#: [Select]
  1.  
  2. using System;
  3. using System.Collections.Generic;
  4. using System.Linq;
  5. using System.Text;
  6. using System.Diagnostics;
  7. using Autodesk.AutoCAD.Runtime;
  8. using Autodesk.AutoCAD.DatabaseServices;
  9. using Autodesk.AutoCAD.EditorInput;
  10. using Autodesk.AutoCAD.ApplicationServices;
  11.  
  12. namespace Duh
  13. {
  14.    public static class AbortThisExample
  15.    {
  16.       static int iterations = 1000;
  17.  
  18.       [CommandMethod( "ABORT_THIS" )]
  19.       public static void AbortThisCommand()
  20.       {
  21.          Document doc = Application.DocumentManager.MdiActiveDocument;
  22.  
  23.          PromptIntegerOptions peo = new PromptIntegerOptions( "\nIterations: " );
  24.          peo.AllowZero = false;
  25.          peo.AllowNegative = false;
  26.          peo.DefaultValue = iterations;
  27.          peo.UseDefaultValue = true;
  28.          PromptIntegerResult pir = doc.Editor.GetInteger( peo );
  29.          if( pir.Status != PromptStatus.OK )
  30.             return;
  31.          iterations = pir.Value;
  32.  
  33.          Stopwatch st = Stopwatch.StartNew();
  34.          for( int i = 0; i < iterations; i++ )
  35.          {
  36.             using( Transaction tr = doc.TransactionManager.StartTransaction() )
  37.             {
  38.                BlockTable bt = (BlockTable) tr.GetObject(
  39.                   doc.Database.BlockTableId,
  40.                   OpenMode.ForRead, false, false );
  41.                tr.Abort();
  42.             }
  43.          }
  44.          st.Stop();
  45.  
  46.          doc.Editor.WriteMessage( "\nTime to abort {0} transactions: {1} ms",
  47.             iterations, st.ElapsedMilliseconds );
  48.          
  49.          st = Stopwatch.StartNew();
  50.          for( int i = 0; i < iterations; i++ )
  51.          {
  52.             using( Transaction tr = doc.TransactionManager.StartTransaction() )
  53.             {
  54.                BlockTable bt = (BlockTable) tr.GetObject(
  55.                   doc.Database.BlockTableId,
  56.                   OpenMode.ForRead, false, false );
  57.                tr.Commit();
  58.             }
  59.          }
  60.          st.Stop();
  61.  
  62.          doc.Editor.WriteMessage( "\nTime to commit {0} transactions: {1} ms",
  63.             iterations, st.ElapsedMilliseconds );
  64.       }
  65.    }
  66. }
  67.  
  68.  
« Last Edit: February 26, 2013, 04:21:49 AM by TT »

Jeff H

  • Needs a day job
  • Posts: 6144
Re: Abort This
« Reply #1 on: February 28, 2013, 11:21:25 AM »
Thanks Tony for the example.

Do you think the GetObject() methods should be overridden to throw an error if OpenWrite is passed in?
« Last Edit: February 28, 2013, 12:25:14 PM by Jeff H »

BRad

  • Guest
Re: Abort This
« Reply #2 on: February 28, 2013, 12:29:16 PM »
Tony,
If time permits, could you please expound on what is happening under the hood with overloading StartReadOnlyTransaction with both a TransactionManager and AcApTransactionManager.  These are the kinds of concepts that are holding me (and probably others) back in intermediate land.
Thanks in advance if you can, understand otherwise.
BRad

It's Alive!

  • Retired
  • Needs a day job
  • Posts: 8659
  • AKA Daniel
Re: Abort This
« Reply #3 on: February 28, 2013, 04:30:18 PM »
Granted commit is faster, just wondering in said employee had a reason to abort read-only transactions over commit?

How do your times compare with using StartOpenCloseTransaction or ObjectId.Open within a using statement?

Also, isn’t there a bit more overhead using the Document transaction over the Database Transaction.?... as I think it calls FlushGraphics on commit… I could be wrong though.

Cheers

Jeff H

  • Needs a day job
  • Posts: 6144
Re: Abort This
« Reply #4 on: February 28, 2013, 06:01:10 PM »
Granted commit is faster, just wondering in said employee had a reason to abort read-only transactions over commit?

How do your times compare with using StartOpenCloseTransaction or ObjectId.Open within a using statement?

Also, isn’t there a bit more overhead using the Document transaction over the Database Transaction.?... as I think it calls FlushGraphics on commit… I could be wrong though.

Cheers


That is what I thought but I copied code from Tony's post here
http://forums.autodesk.com/t5/NET/Seeking-Advice-to-Better-Routine/m-p/3787758#M33614
and just and made another command from it and just changed it to use the Document's Transaction, but they both performed almost the same comparing difference between committing and aborting, but the document's transaction performed slightly better.




fentonwebb

  • Guest
Re: Abort This
« Reply #5 on: February 28, 2013, 06:57:43 PM »
Hey TT!

thanks for your information.

I think it's rare for people to call Abort for read only operations (looking at the API it's not something you would consider calling for a read only operation), but you do make a valid point in my opinion. Therefore, I've used your sample code to simply log a Change Request to make Transaction.Abort() understand that if there are no write-open objects that need aborting, then a Commit() will be called instead.

Remember, if it's performance you are worried about then you should be using StartOpenCloseTransaction, or even faster again is normal ObjectId.Open/Close. Personally, I recommend ObjectId.Open/Close().

Thanks again, TT

TheMaster

  • Guest
Re: Abort This
« Reply #6 on: March 01, 2013, 02:04:39 AM »
Hey TT!

thanks for your information.

I think it's rare for people to call Abort for read only operations (looking at the API it's not something you would consider calling for a read only operation), but you do make a valid point in my opinion. Therefore, I've used your sample code to simply log a Change Request to make Transaction.Abort() understand that if there are no write-open objects that need aborting, then a Commit() will be called instead.

Remember, if it's performance you are worried about then you should be using StartOpenCloseTransaction, or even faster again is normal ObjectId.Open/Close. Personally, I recommend ObjectId.Open/Close().

Thanks again, TT

Hi Fenton, and thanks for taking the time to comment.

I think your proposal is an excellent idea.

In my comments, I was going by what we were told once a long time ago on the Autodesk discussion groups by Albert S., about avoiding numerous calls to Transaction.Abort(), which of course predated the OpenCloseTransaction.

The OpenCloseTransaction is certainly faster, but it doesn't manage anything besides changes to database objects. For example, it doesn't control changes to the application or drawing editor environment, system variables, etc, as normal Transactions do.

The other issue is that there are some cases where database objects must be transaction resident. If I'm not mistaken, accessing dynamic block properties requires the owning BlockReference to be transaction-resident. Bummer.


TheMaster

  • Guest
Re: Abort This
« Reply #7 on: March 01, 2013, 02:19:13 AM »
Thanks Tony for the example.

Do you think the GetObject() methods should be overridden to throw an error if OpenWrite is passed in?

Well, this is going a bit overboard, and was really just an experiment, but the SmartTransaction class shown below, will automatically switch to aborting when it is disposed, if there is a call to GetObject() with the OpenMode argument set to OpenMode.ForWrite.

Remember, this was only an experiment :laugh:

Code - C#: [Select]
  1.  
  2. public class SmartTransaction : CustomTransaction
  3. {
  4.    bool isReadOnly = true;
  5.  
  6.    public SmartTransaction( Transaction trans )
  7.       : base( trans )
  8.    {
  9.    }
  10.  
  11.    /// <summary>
  12.    ///
  13.    /// Property: bool IsReadOnly (default = true)
  14.    ///
  15.    /// If true, the transaction will be committed when it is
  16.    /// disposed. Otherwise, the transaction is aborted when
  17.    /// it is disposed (default behavior). The initial value
  18.    /// of this property is true, and will become false the
  19.    /// first time an object is obtained via GetObject() and
  20.    /// the OpenMode argument is OpenMode.ForWrite, or if a
  21.    /// a new object is added via AddNewlyCreatedDBObject().
  22.    ///
  23.    /// Once the value has become true and changes have been
  24.    /// made, the transaction acts like a normal transaction,
  25.    /// requiring a call to Commit() to prevent the instance
  26.    /// from aborting when it is disposed.
  27.    ///
  28.    /// Typically, you set this property to false just before
  29.    /// modifying the drawing and/or the environment, and want
  30.    /// those changes to be rolled back if an exception occurs.
  31.    ///
  32.    /// If your operations are purely read-only, and you do not
  33.    /// modify the database or environment, you would leave the
  34.    /// value of this property set to true, and not bother to
  35.    /// call Commit() (the transaction is committed when it is
  36.    /// disposed in that case).
  37.    ///
  38.    /// </summary>
  39.  
  40.    public bool IsReadOnly
  41.    {
  42.       get
  43.       {
  44.          return isReadOnly;
  45.       }
  46.       set
  47.       {
  48.          isReadOnly = value;
  49.       }
  50.    }
  51.  
  52.    public override DBObject GetObject( ObjectId id, OpenMode mode )
  53.    {
  54.       return this.GetObject( id, mode, false, false );
  55.    }
  56.  
  57.    public override DBObject GetObject( ObjectId id, OpenMode mode, bool openErased )
  58.    {
  59.       return this.GetObject( id, mode, openErased, false );
  60.    }
  61.  
  62.    public override DBObject GetObject( ObjectId id, OpenMode mode, bool openErased, bool forceOpenOnLockedLayer )
  63.    {
  64.       if( mode == OpenMode.ForWrite && isReadOnly )
  65.          isReadOnly = false;
  66.       return base.GetObject( id, mode, openErased, forceOpenOnLockedLayer );
  67.    }
  68.  
  69.    public override void AddNewlyCreatedDBObject( DBObject obj, bool add )
  70.    {
  71.       if( add && isReadOnly )
  72.          isReadOnly = false;
  73.       base.AddNewlyCreatedDBObject( obj, add );
  74.    }
  75.  
  76.    protected override void Dispose( bool disposing )
  77.    {
  78.       if( disposing )
  79.       {
  80.          if( base.AutoDelete )
  81.          {
  82.             if( isReadOnly )
  83.                this.Commit();
  84.             else
  85.                this.Abort();
  86.          }
  87.       }
  88.       base.Dispose( disposing );
  89.    }
  90. }
  91.  
  92.  

TheMaster

  • Guest
Re: Abort This
« Reply #8 on: March 01, 2013, 02:23:18 AM »
Tony,
If time permits, could you please expound on what is happening under the hood with overloading StartReadOnlyTransaction with both a TransactionManager and AcApTransactionManager.  These are the kinds of concepts that are holding me (and probably others) back in intermediate land.
Thanks in advance if you can, understand otherwise.
BRad

There's 2 TransactionManager types in the AutoCAD API. One is in the DatabaseServices namespace (the one returned by the Database.TransactionManager property), and the other is in the ApplicationServices namespace (and is the type returned by the Document.TransactionManager property).

The extension method is overloaded so it can be applicable to both.

TheMaster

  • Guest
Re: Abort This
« Reply #9 on: March 01, 2013, 02:30:29 AM »
Granted commit is faster, just wondering in said employee had a reason to abort read-only transactions over commit?

How do your times compare with using StartOpenCloseTransaction or ObjectId.Open within a using statement?

Also, isn’t there a bit more overhead using the Document transaction over the Database Transaction.?... as I think it calls FlushGraphics on commit… I could be wrong though.

Cheers

Hi Dan.  The Abort() method has major overhead because it is causing a display update of some sort (I see the display flickering when running the test).

The OpenCloseTransaction's Abort() method has no overhead like that, it's essentially the same as Commit().  I haven't noticed much difference between the document and database transactions, they both have the same overhead with Abort(), and Commit() seems to be almost the same as well.


Alexander Rivilis

  • Bull Frog
  • Posts: 214
  • Programmer from Kyiv (Ukraine)
Re: Abort This
« Reply #10 on: March 01, 2013, 04:57:07 AM »
...Therefore, I've used your sample code to simply log a Change Request to make Transaction.Abort() understand that if there are no write-open objects that need aborting, then a Commit() will be called instead...
I hope that like in ObjectARX (i.e. AcDbTransactionManager::abortTransaction ())

TheMaster

  • Guest
Re: Abort This
« Reply #11 on: March 01, 2013, 09:25:26 AM »
...Therefore, I've used your sample code to simply log a Change Request to make Transaction.Abort() understand that if there are no write-open objects that need aborting, then a Commit() will be called instead...
I hope that like in ObjectARX (i.e. AcDbTransactionManager::abortTransaction ())

Fenton's proposal sounds good on the surface, but from what I can tell, the outer-most transaction acts like it is implicitly an undo group, and will roll back anything, including changes to the application and document state (for example, it will undo changes to system variables like CMDECHO), so I'm not so sure it would be as simple as looking to see if any objects were opened for write through the transaction, it would also have to check if anything that is 'undo-able' was done during the transaction's life.

Jeff H

  • Needs a day job
  • Posts: 6144
Re: Abort This
« Reply #12 on: March 01, 2013, 10:07:29 AM »
docs
Quote
If an object is modified with the open and close mechanism while a transaction is active, changes to it will be rolled back with the transaction during UNDO operations.
So it includes system variables also.

Also if you use a custom command will everything get undone unless you use flag it with NoUndoMarker?

owenwengerd

  • Bull Frog
  • Posts: 451
Re: Abort This
« Reply #13 on: March 01, 2013, 11:14:16 AM »
Therefore, I've used your sample code to simply log a Change Request to make Transaction.Abort() understand that if there are no write-open objects that need aborting, then a Commit() will be called instead.

This is a mistake. If the programmer wants to Abort, the transaction should not second guess the programmer's intentions -- it should Abort. Better would be to optimize Abort so that it doesn't waste time, but only does cleanup or rollbacks that are needed (perhaps by monitoring what happened while it was active, perhaps by comparing the current state with the initial state).

TheMaster

  • Guest
Re: Abort This
« Reply #14 on: March 02, 2013, 09:47:03 AM »
Therefore, I've used your sample code to simply log a Change Request to make Transaction.Abort() understand that if there are no write-open objects that need aborting, then a Commit() will be called instead.

This is a mistake. If the programmer wants to Abort, the transaction should not second guess the programmer's intentions -- it should Abort. Better would be to optimize Abort so that it doesn't waste time, but only does cleanup or rollbacks that are needed (perhaps by monitoring what happened while it was active, perhaps by comparing the current state with the initial state).

Hi Owen.

I think the point you raise really has a lot to do with what Commit() does when there has been no changes. As far as I can tell, it does nothing.  So, the intent underlying Fenton's proposal would seem to be more simply, that Abort() should also do nothing when there's been no change (to the state of the application, system variables or anything else that is undo-able).

So, if it's true that when nothing has changed, Commit() does nothing, then I don't think I would agree that Abort() should not also do the same, whether it is actually deferring to the same code called by Commit(), or not, the outcome is the same.

I do agree that the view update that seems to be responsible for most of the time consumed by Abort() even when nothing's changed is avoidable.

This thread began as a result of an email I got, asking for an opinion on whether the code that was handed out at AU by an Autodesk employee, that consistently aborted transactions when nothing was changed, was a good practice.  I don't think it is, because if there was any kind of user-interaction while the transaction was active, the user could have changed the state of the environment (system variables, transparent view changes, etc.), and transparent user changes should almost never be rolled back.

« Last Edit: March 02, 2013, 10:05:06 AM by TT »

owenwengerd

  • Bull Frog
  • Posts: 451
Re: Abort This
« Reply #15 on: March 02, 2013, 11:10:17 AM »
I think you make a valid point about Abort using the same underlying code as Commit, but in my opinion there is (or should be) a *logical* distinction at least, even if the implementation shares code under the hood. I view a transaction as a single atomic "action". From that perspective, the logical purpose of Abort is not to roll back changes per se, but to prevent them ever occurring. The fact that under the hood this is managed by actually making the changes one by one, then rolling them back in Abort, should be an imnplementation detail. I think logically, system state changes that are side effects of (or "belong to") the transaction should participate in the transaction model -- but I don't think wheel zooming, for example, "belongs" to the transaction.

In any case, I think we agree that Abort is not the logically correct way to end a read-only transaction, even if in practice an Abort perhaps ends up behaving identically to a Commit. A transaction, whether writes occurred or not, should be aborted when something prevents its successful completion such that all the changes and side effects that belong to the transaction are never (logically) performed.

My concern about Fenton's response is that I sensed a willingmess to ignore the logical purpose of Commit and Abort in favor of "just making it work".