Re: FYI: unwind_protect improved

[Jaroslav Hajek]

On Thu, Jun 25, 2009 at 12:03 PM, Jaroslav Hajek<address@hidden> wrote:
> On Tue, Jun 23, 2009 at 8:48 AM, Jaroslav Hajek<address@hidden> wrote:
>> hi all,
>>
>> to solve certain problems, including
>> http://www.nabble.com/3.2.0-compile-error-on-FreeBSD-7.2-amd64-(sig_atomic_t-!%3D-int-on-amd64)-td24124769.html#a24124769
>> ,
>> I have improved the design of the unwind_protect class somewhat.
>>
>> Summary:
>>
>> There's a generic unwind_protect::protect_var method able to handle a
>> variable of any assignable & copyable class.
>> unwind_protect::protect_mem is also provided though it will probably
>> be very seldom used (given that you can protect a NDArray or whatever
>> with protect_var).
>>
>> using string tags for unwind_protect frames is now deprecated:
>>
>>  unwind_protect::begin_frame ("tree_evaluator::visit_simple_for_command");
>> ...
>>  unwind_protect::run_frame ("tree_evaluator::visit_simple_for_command");
>>
>> In particular, if you misspelled the string in run_frame, it would
>> silently run all frames down the stack (!).
>> The proper usage is now:
>>
>>  unwind_protect::frame_id_t uwp_frame = unwind_protect::begin_frame ();
>> ...
>>  unwind_protect::run_frame (uwp_frame);
>>
>> which allocates the frame info as a local variable (in fact, it's
>> simply the stacksize at the moment of query). This is also somewhat
>> more efficient, as there's no string manipulation when processing the
>> stack, neither is there need to store them in the stack.
>>
>> The old tags mechanism is still supported, but the methods are marked
>> as deprecated. They should be removed at some point.
>>
>> It is also possible to protect a single variable locally, if
>> performance is of particular concern:
>>
>> {
>>   unwind_protect::restore_var<bool> flag_protect (flag); // will
>> auto-restore flag at end of block
>>   flag = temporary value;
>>   ...
>> }
>>
>> this is more efficient because the compiler can inline everything.
>>
>> Comments?
>>
>
> Unfortunately, I wasn't satisfied. The main issue was that
> unwind_protect::discard leaked memory (which was true even before my
> changes), and when attempting to remove that defect, it got a bit
> clunky.
>
> Hence, I once again redesigned the unwind_protect internals. The basis
> is now formed by polymorphic unwind_protect::elem class, which has a
> virtual run() method and a virtual destructor.
>
> The stack methods simply handle the stack and call these methods 
> appropriately.
>
> The advantage is that the design is now very clear and OOPishly
> flexible - unwind_protect provides stock subclasses of elem (fcn_elem,
> restore_var_elem, action_var_elem), more can be added, or users can
> subclass their own. Less need for reinterpret_cast, for instance.
>
> The downside is a possible performance impact - processing an element
> now requires two virtual method calls rather than one call through
> pointer. However, here I felt (amazingly enough :) that the (supposed)
> loss is an acceptable tradeoff for the obvious gain in clarity and
> flexibility. Besides, compilers can sometimes do a lot of
> optimizations with virtual method calls.
>
> I replaced the unwind_protect::add calls binding to variables in local
> scope by unwind_protect::add_action_var, which packs a function
> together with an argument.
> The only place left is in mex.cc:call_mex (passing a pointer to local
> variable context). I'm not sure what is best here.
> The class mex has a trivial copy constructor, yet non-trivial
> destructor, which is always suspicious. Hence I'm not sure whether
> it's safe to copy the variable.
>
> regards
>

So, after some more thinking & tweaking, I ended up with the following:

unwind_protect now supports these operations:

* unwind_protect::add_fcn (func)
   calls func ().

* unwind_protect::add_fcn (func, val)
  calls func (val).

* unwind_protect::add_method (obj, &class::method)
  calls obj->method ().

* unwind_protect::add_delete (ptr)
  calls delete ptr.

* unwind_protect::protect_var (var)
   restores the value of (non-local) variable var.

* unwind_protect::protect_var (var)
   restores the value of (non-local) variable var to the value val.

armed by these, I simplified the unwind_protect handling all over the
sources. A plausible number of assistant wrappers have gone away.

comments? ideas for more?

I'd say the unwind_protect mechanism is now more user friendly. I
dunno whether it is actually measurably slower, but I doubt it.

regards

-- 
RNDr. Jaroslav Hajek
computing expert & GNU Octave developer
Aeronautical Research and Test Institute (VZLU)
Prague, Czech Republic
url: www.highegg.matfyz.cz