Re: [boost] [optional] generates unnessesary code for trivial types

Subject: Re: [boost] [optional] generates unnessesary code for trivial types
From: Hite, Christopher (Christopher.Hite_at_[hidden])
Date: 2012-02-01 11:53:22

> On 27.1.2012. 11:32, Domagoj Saric wrote:

> a) the lifetime management bool was changed into a properly typed pointer (this
>     actually takes the same amount of space while it provides a no-op get_ptr()
>     member function as well as easier debugging as the contents of optional can
>     now clearly be seen through the pointer, as opposed to gibberish in an opaque
>    storage array)
I'd support this only if were configurable.  It takes more space for small or
non-word-aligned data.  It might be more expensive on some systems to calculate
the address and store it.

I did think that about defaulting to int_fast8_t for the bool if its alignment>=
alignment of T.  On my x86 system it's still 1 byte though.  On some system it
might help.

It would also break has_trivial_copy.  If someone was naughty and memcopied them,
the new version would lead to a very hard to find bug.

As for the debugger the new C++ allows for a union to contain a class.  So if
a placeholder implemention using such a union would show the data in debug.

> d) skips redundant/dead stores of marking itself as uninitialised [including but
>     limited to, in its destructor (if it has one)]
> e) streamlined internal assign paths to help the compiler avoid unnecessary
>     branching
Sounds like what I'm after.

> f) added direct_create() and direct_destroy() member functions that allow the
>     user to bypass the internal lifetime management (they only assert correct
>     usage) in situations where the user's own external logic already implicitly
>     knows the state of the optional
Sounds good.  I also wanted these.

> g) optional now declares and defines a destructor only if the contained type has
>     a non-trivial destructor (this prevents the compiler from detecting false EH
>     states and thus generating bogus EH code)
Yes, that's what I want.

> h) optional marks itself as uninitialised _before_ calling the contained
>     object's destructor (this makes it a little more robust in race conditions;
>     it is of course not a complete solution for such scenarios, those require
>     external "help" and/or (m)-reference counting to be implemented)
Seems to contradict (g).  I'd support something like that only if it can be
configured out.  Maybe there's some case completely out of optional's scope
where you use atomic ops.

If you factor out the aligned storage you can build something else that does
ref-counting or a thread safe state machine or whatever.

> i) extracted the "placeholder" functionality into a standalone class (basically
>     what would be left of optional<> if the lifetime management "bool" member and
>     logic was removed) so that it can be reused (e.g. for singleton like classes,
>     or when more complex custom lifetime management is required)
I 100% agree with this.  I think there should be one placeholder implementation.
I think boost::function should use it as well.  I think it may be useful to users.

> k) the lifetime management pointer is now stored after the actual contained
>     object (this helps in avoiding more complex/offset addressing when accessing
>     optionals through pointers w/o checking whether they are initialised)
Seems weird.  If the front of T is more likely to be used (and old char buffer),
your pointer may wind up in a different cache line.

> o) avoid branching in assignment and copy construction of optionals that hold
>     PODs smaller than N * sizeof( void * ) where N is some small number
Again it would be cool if the user had control over this.

I'm going to have to check out your code.

So the big thing I take away from all this it would be really nice if some things
were configurable.  How do we do that without breaking code?

Changing the signature to optional<T,Properties=optional_traits<T> >, might
break code that uses boost::optional as a template template parameter.

You could just refer to optional_traits inside and force the user to specialize
it for his T, but that could create violations of the one definition rule.

Also is it OK for optional to depend on enable_if/SFINAE and type traits?

Chris