Re: [trans-mem] Move state from gtm_thread to gtm_transaction

[Torvald Riegel <triegel at redhat dot com>]

On Thu, 2011-08-04 at 11:55 -0700, Richard Henderson wrote:
> On 08/04/2011 09:22 AM, Torvald Riegel wrote:
> > On Thu, 2011-08-04 at 08:43 -0700, Richard Henderson wrote:
> >> On 08/03/2011 04:04 AM, Torvald Riegel wrote:
> >>>     Move local_tid from gtm_thread to gtm_transaction.
> >>>     
> >>>     	* config/generic/tls.h (gtm_thread): Move local_tid from here ...
> >>>     	* libitm_i.h (local_tid): ... to here.
> >>>     	* beginend.cc (GTM::gtm_transaction::begin_transaction): Same.
> >>>     	(GTM::gtm_transaction::operator new): Set up gtm_thread if necessary.
> >>
> >> This seems very wrong.  Why?
> > 
> > What seems wrong?
> > 
> > local_tid is per thread and there is one gtm_transaction object per
> > thread, so moving it is correct, or not?
> > The purpose of this is to not having to access gtm_thread anymore in
> > begin. And it's a step towards merging gtm_thread and gtm_transaction
> > completely.
> > 
> > Do you agree?
> 
> If you're going to merge gtm_thread and gtm_transaction, why
> are you moving things away from gtm_thread?

I believe the per-thread object should be gtm_transaction, not
gtm_thread. Regarding the name, "transaction" makes more sense to me
because it has almost no uses besides transaction-related stuff.
Second, I guess we'd have to keep it in libitm_i.h anyway, and not in
tls.h. Therefore, it seems to be the right thing to do to put it
together with the related stuff, even if we rename gtm_transaction to
gtm_thread later, or move it out of libitm_i.h (why would we? where
to?).

> As for "not having to access gtm_thread", the non-glibc case
> for accessing gtm_txn is to pull the value out of gtm_thread.

I'm aware of that, but non-glibc is not the common case we're optimizing
for. If so, I believe we'd have kept the tx param in all the _ITM_* load
and store functions.
Either way, when gtm_thread and gtm_transaction have been merged (and
I'm thinking about merging into gtm_transaction here...), this issue
should also have gone away.

I'm planning to come up with a separate patch for the actual merge. I
first need to think a bit more about whether the destruction can be done
with just a destructor of the TLS object, what happens when other
object's destructors use transactions, etc. Even with one txn per
thread, we still need to deregister it and release its dynamically
allocated memory. Perhaps adding a destruction-phase mode would help, in
which transactions are always destructed after they've run.

Torvald