On Thu, 2011-07-28 at 09:02 -0700, Richard Henderson wrote:
> > Add information to dispatch about closed nesting and uninstrumented
> > code.
> >
> > * dispatch.h (GTM::abi_dispatch): Add
> > can_run_uninstrumented_code and
> > closed_nesting flags, as well as a closed nesting alternative.
> > * method-serial.cc: Same.
>
> Nearly...
>
> > + virtual abi_dispatch* closed_nesting_alternative()
> > + {
> > + // For nested transactions with an instrumented code path, we can do
> > + // undo logging.
> > + return GTM::dispatch_serial();
>
> Surely you really mean dispatch_serial_ul here?
> Otherwise ok.
No, this is correct because it calls the factory function in libitm_i.h.
However, the classes in method-serial.cc were named differently than
those factory functions, so I renamed them like this:
-class serial_dispatch : public abi_dispatch
+class serialirr_dispatch : public abi_dispatch
-class serial_dispatch_ul : public abi_dispatch
+class serial_dispatch : public abi_dispatch
This should avoid confusion in the future.
> > Add closed nesting as restart reason.
> >
> > * libitm_i.h: Add closed nesting as restart reason.
> > * retry.cc (GTM::gtm_transaction::decide_retry_strategy): Same.
>
> Ok, except
>
> > + if (r == RESTART_CLOSED_NESTING) retry_serial = true;
>
> Coding style. THEN statement on the next line, even for small THEN.
Will try to keep that in mind ...
> > Make flat nesting the default, use closed nesting on demand.
> >
> > * local.cc (gtm_transaction::rollback_local): Support closed
> > nesting.
> > * eh_cpp.cc (GTM::gtm_transaction::revert_cpp_exceptions): Same.
> > * dispatch.h: Same.
> > * method-serial.cc: Same.
> > * beginend.cc (GTM::gtm_transaction::begin_transaction): Change
> > to
> > flat nesting as default, and closed nesting on demand.
> > (GTM::gtm_transaction::rollback): Same.
> > (_ITM_abortTransaction): Same.
> > (GTM::gtm_transaction::restart): Same.
> > (GTM::gtm_transaction::trycommit): Same.
> > (GTM::gtm_transaction::trycommit_and_finalize): Removed.
> > (choose_code_path): New.
> > (GTM::gtm_transaction_cp::save): New.
> > (GTM::gtm_transaction_cp::commit): New.
> > * query.cc (_ITM_inTransaction): Support flat nesting.
> > * libitm_i.h (GTM::gtm_transaction_cp): New helper struct for
> > nesting.
> > (GTM::gtm_transaction): Support flat and closed nesting.
> > * alloc.cc (commit_allocations_2): New.
> > (commit_cb_data): New helper struct.
> > (GTM::gtm_transaction::commit_allocations): Handle nested
> > commits/rollbacks.
> > * libitm.texi: Update user action section, add description of
> > nesting.
>
> Nearly...
>
> > + abi_dispatch *cn_disp = disp->closed_nesting_alternative();
> > + if (cn_disp)
> > + {
> > + disp = cn_disp;
> > + set_abi_disp(disp);
> > + }
>
> Don't we need to fini the old disp? Seems there's a leak here, though
> not visible until we re-instate the non-serial methods.
Yes, probably. However, one of the next steps on my refactoring list is
to document and change the TM method lifecycle callbacks. This will
include grouping several compatible methods (ie, those that can run
together) into method sets (e.g., global lock, multiple locks).
Switching a method within the current method set would then require no
fini(), whereas switching the method set would require a more
heavy-weight callback.
I have put the case you raised on my to-do list, and will revisit it
when working on these lifecycle management changes.
>
> > + if (!(tx->state & STATE_IRREVOCABLE)) ret |= a_saveLiveVariables;
Fixed.
Committing to branch, together with the two more recent changes.
Torvald
