[trans-mem] Beginning of refactoring

[Torvald Riegel]

On Tue, 2011-07-19 at 01:22 +0200, Torvald Riegel wrote:
> On Tue, 2011-07-19 at 01:19 +0200, Torvald Riegel wrote:
> > On Sat, 2011-07-09 at 16:30 +0200, Torvald Riegel wrote:
> > > The attached patch makes flat nesting the default, while still
> > > supporting closed nesting on demand (for user-controlled aborts via
> > > __transaction_cancel).
> > > Previously, a new transaction object was created for each nested
> > > transaction. The patch changes this to using one transaction object
> > > during the whole lifetime of a thread and keeping checkpoints of the
> > > transaction state when starting closed nested transactions. This allows
> > > us to easily use flat nesting and decreases the transaction start/commit
> > > overheads to some extent.
> > > 
> > > OK for branch?
> > 
> > I split the patch, as requested.
> > 
> > First three parts are smaller changes:
> > 
> > patch1: New erase method and placement new for aatree.
> > patch2: Change pr_hasElse to the value specified in the ABI.
> > patch3: Add information to dispatch about closed nesting and
> > uninstrumented code.
> > 
> > Then, in preparation for the following flat-transaction object design:
> > patch4: Use vector instead of list to store user actions.
> > patch5: Add closed nesting as restart reason.
> > 
> > And the actual core change:
> > patch6: Make flat nesting the default, use closed nesting on demand.
> > 
> > This last patch is still rather large, but it is one logical piece.
> > beginend.cc has most changes, but splitting this one up would rather
> > create incomplete intermediate steps than make the direction of this
> > whole patch clear.
> > 
> > As discussed offline, reducing the one extra copying of jmpbuf for
> > actual closed-nested transactions can be addressed in a future patch by
> > modifying _ITM_beginTransaction, which could additionally remove the
> > extra copying from stack to jmpbuf for all transactions as it existed
> > before this patch here.
> > 
> > Ok for branch, or is further splitting required?
> > 
> 
> This time with patches attached, sorry.

Attached are two small (in size) fixes for this patch set.

patch7: gtm_transaction::decide_begin_dispatch() gets the transaction
properties from the caller instead of reading from
gtm_transaction::prop, which might not have been updated by the caller
yet.

patch8: Fix nesting level reset when rolling back the outermost
transaction. Before, this was incorrectly reset to zero, which caused
transactions to not get committed. This didn't show up in previous
testing because previously, only serial-mode TM methods were available.

OK for branch, together with the previous patches?
-------------- next part --------------
commit f3d24b9933d2124fe330e0513f14ce0711402e88
Author: Torvald Riegel <triegel@redhat.com>
Date:   Tue Jul 26 20:28:54 2011 +0200

    Fix gtm_transaction::decide_begin_dispatch().
    
    	* retry.cc (GTM::gtm_transaction::decide_begin_dispatch): Get transaction
    	properties from the caller instead of from the transaction object.
    	* libitm_i.h: Same.
    	* beginend.cc (GTM::gtm_transaction::begin_transaction): Same.

diff --git a/libitm/beginend.cc b/libitm/beginend.cc
index 6023390..417b41a 100644
--- a/libitm/beginend.cc
+++ b/libitm/beginend.cc
@@ -187,7 +187,7 @@ GTM::gtm_transaction::begin_transaction (uint32_t prop, const gtm_jmpbuf *jb)
         tx->state = (STATE_SERIAL | STATE_IRREVOCABLE);
 
       else
-        disp = tx->decide_begin_dispatch ();
+        disp = tx->decide_begin_dispatch (prop);
 
       if (tx->state & STATE_SERIAL)
         {
diff --git a/libitm/libitm_i.h b/libitm/libitm_i.h
index 4cc0a70..093fc0e 100644
--- a/libitm/libitm_i.h
+++ b/libitm/libitm_i.h
@@ -218,7 +218,7 @@ struct gtm_transaction
 
   // In retry.cc
   void decide_retry_strategy (gtm_restart_reason);
-  abi_dispatch* decide_begin_dispatch ();
+  abi_dispatch* decide_begin_dispatch (uint32_t prop);
 
   // In method-serial.cc
   void serialirr_mode ();
diff --git a/libitm/retry.cc b/libitm/retry.cc
index bfca2ca..91778e6 100644
--- a/libitm/retry.cc
+++ b/libitm/retry.cc
@@ -77,12 +77,12 @@ GTM::gtm_transaction::decide_retry_strategy (gtm_restart_reason r)
 
 
 // Decides which TM method should be used on the first attempt to run this
-// transaction, setting this->disp accordingly.
+// transaction.
 // serial_lock will not have been acquired if this is the outer-most
 // transaction. If the state is set to STATE_SERIAL, the caller will set the
 // dispatch.
 GTM::abi_dispatch*
-GTM::gtm_transaction::decide_begin_dispatch ()
+GTM::gtm_transaction::decide_begin_dispatch (uint32_t prop)
 {
   // ??? Probably want some environment variable to choose the default
   // STM implementation once we have more than one implemented.
-------------- next part --------------
commit f85b6ce7b67cecfd9c056620c7ef2aaa173fe5bc
Author: Torvald Riegel <triegel@redhat.com>
Date:   Wed Jul 27 12:27:10 2011 +0200

    Fix nesting level reset when rolling back the outermost transaction.
    
    	* beginend.cc (GTM::gtm_transaction::rollback): Fix nesting level
    	rollback.

diff --git a/libitm/beginend.cc b/libitm/beginend.cc
index 417b41a..ba90af2 100644
--- a/libitm/beginend.cc
+++ b/libitm/beginend.cc
@@ -291,6 +291,7 @@ GTM::gtm_transaction::rollback (gtm_transaction_cp *cp)
     }
   else
     {
+      // Roll back to the outermost transaction.
       // Restore the jump buffer and transaction properties, which we will
       // need for the longjmp used to restart or abort the transaction.
       if (parent_txns.size() > 0)
@@ -298,9 +299,11 @@ GTM::gtm_transaction::rollback (gtm_transaction_cp *cp)
           jb = parent_txns[0].jb;
           prop = parent_txns[0].prop;
         }
-      // Reset the transaction. Do not reset state, which is handled by the
-      // callers.
-      nesting = 0;
+      // Reset the transaction. Do not reset this->state, which is handled by
+      // the callers. Note that we reset the transaction to the point after
+      // having executed begin_transaction (we will return from it), so the
+      // nesting level must be one, not zero.
+      nesting = 1;
       parent_txns.clear();
     }