[Bug libstdc++/98978] New: Consider packing _M_Engaged in the tail padding of T in optional<>

[Bug libstdc++/98978] New: Consider packing _M_Engaged in the tail padding of T in optional<>

[andysem at mail dot ru]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98978

            Bug ID: 98978
           Summary: Consider packing _M_Engaged in the tail padding of T
                    in optional<>
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: andysem at mail dot ru
  Target Milestone: ---

Using std::optional with some types may considerably increase object sizes
since it adds alignof(T) bytes worth of overhead. Sometimes it is possible to
avoid this overhead if the flag indicating presence of the stored value
(_M_Engaged in libstdc++ sources) is placed in the tail padding of the T
object. This can be done if std::optional constructs an object of a type that
derives from T, which has an additional bool data member that is initialized to
true upon construction. The below code roughly illustrates the idea:

template< typename T >
struct _Optional_payload_base
{
  struct _PresentT : T
  {
    const bool _M_Engaged = true;

    // Forwarding ctors and other members
  };

  static constexpr size_t engaged_offset = offsetof(_PresentT, _M_Engaged);

  struct _AbsentT
  {
    unsigned char _M_Offset[engaged_offset];
    const bool _M_Engaged = false;
  };

  union _Storage
  {
    _AbsentT _M_Empty;
    _PresentT _M_Value;

    _Storage() : _M_Empty() {}

    // Forwarding ctors and other members
  };

  _Storage _M_payload

  bool is_engaged() const noexcept
  {
    return *reinterpret_cast< const bool* >(reinterpret_cast< const unsigned
char* >(&_M_payload) + engaged_offset);
  }
};

The above relies on some implementation details, such as:

- offsetof works for the type T. It does for many types in gcc, beyond what is
required by the C++ standard. Maybe there is a way to avoid offsetof, I just
didn't immediately see it.
- The location of _M_Engaged in both _PresentT and _AbsentT is the same. This
is a property of the target ABI, and AFAICS it should be true at least on x86
psABI and I think Microsoft ABI.

The above will only work for non-final class types, for other types, and where
the above requirements don't hold true, the current code with a separate
_M_Engaged flag would work.

[Bug libstdc++/98978] Consider packing _M_Engaged in the tail padding of T in optional<>

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98978

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |ABI

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
This would be an ABI break, and so not going to happen.

[Bug libstdc++/98978] Consider packing _M_Engaged in the tail padding of T in optional<>

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98978

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
If we were going to do this, we could also make std::optional<bool> occupy a
single byte, using one bit for the value and one for the engaged flag.

[Bug libstdc++/98978] Consider packing _M_Engaged in the tail padding of T in optional<>

[andysem at mail dot ru]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98978

--- Comment #3 from andysem at mail dot ru ---
(In reply to Jonathan Wakely from comment #1)
> This would be an ABI break, and so not going to happen.

Is there no way to improve standard components implementation? I'd imagine you
could provide the new implementation in the new version inline namespace and
still support the old ABI for backward compatibility.

(In reply to Jonathan Wakely from comment #2)
> If we were going to do this, we could also make std::optional<bool> occupy a
> single byte, using one bit for the value and one for the engaged flag.

This would be more problematic as emplace(), value() and operator*() need to
return T&, which would not be possible.

[Bug libstdc++/98978] Consider packing _M_Engaged in the tail padding of T in optional<>

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98978

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to andysem from comment #3)
> Is there no way to improve standard components implementation? I'd imagine
> you could provide the new implementation in the new version inline namespace
> and still support the old ABI for backward compatibility.

Yes ABI changes can be done for the --enable-symvers=gnu-versioned-namespace
configuration, but nobody uses it so anything invasive like this is not worth
the effort.

> This would be more problematic as emplace(), value() and operator*() need to
> return T&, which would not be possible.

Use 0xff to mean disengaged, and 0x00 and 0x01 for engaged. You only need to
return a reference when it's engaged.

[Bug libstdc++/98978] Consider packing _M_Engaged in the tail padding of T in optional<>

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98978

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement
   Last reconfirmed|                            |2021-08-31
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

[Bug libstdc++/98978] Consider packing _M_Engaged in the tail padding of T in optional<>

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98978

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to andysem from comment #3)
> Is there no way to improve standard components implementation? I'd imagine
> you could provide the new implementation in the new version inline namespace
> and still support the old ABI for backward compatibility.

To give a more complete answer: Inline namespaces don't help, that's a myth.

struct X { std::optional<bool> b; };

Now one translation unit has X using the old ABI and one has X using the new
ABI, but they're the same X. The fact that the two versions of optional are in
different namespaces is no help at all. You still have an ABI break.

We did it for std::string and it was a multi-year effort that caused disruption
across the industry. It's not worth doing that again to save a few bytes in
std::optional.

[Bug libstdc++/98978] Consider packing _M_Engaged in the tail padding of T in optional<>

[andysem at mail dot ru]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98978

--- Comment #6 from andysem at mail dot ru ---
(In reply to Jonathan Wakely from comment #5)
> (In reply to andysem from comment #3)
> > Is there no way to improve standard components implementation? I'd imagine
> > you could provide the new implementation in the new version inline namespace
> > and still support the old ABI for backward compatibility.
> 
> To give a more complete answer: Inline namespaces don't help, that's a myth.
> 
> struct X { std::optional<bool> b; };
> 
> Now one translation unit has X using the old ABI and one has X using the new
> ABI, but they're the same X. The fact that the two versions of optional are
> in different namespaces is no help at all. You still have an ABI break.
> 
> We did it for std::string and it was a multi-year effort that caused
> disruption across the industry. It's not worth doing that again to save a
> few bytes in std::optional.

You could include a hash of all members' namespaces in the X's mangled name. Or
maybe add an attribute that would propagate a tag to the outer class' mangled
name. Yes, that's also an ABI change in itself, but a useful one that would
allow inline namespaces to become useful.

I think a way to improve standard library components is essential. If not
inline namespaces then something else. Otherwise there is no evolution path of
the standard library components.