Re: FYI: unwind_protect improved

[Jaroslav Hajek]

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

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