search

martinmoene / ring-span-lite

ring-span lite - A C++yy-like ring_span type for C++98, C++11 and later in a single-file header-only library
Boost Software License 1.0
152 stars 12 forks source link

Empty base optimization for m_popper #23

Closed dcolascione closed 3 years ago

dcolascione commented 3 years ago

Right now, popper is a field, m_popper. We store a byte for m_popper even though the popper is usually stateless. If the popper were instead a base class, there would be no size penalty in the case that m_popper has no state.

Quuxplusone commented 3 years ago

IMO, since C++20 introduces [[no_unique_address]] for this purpose, I think it would be a reasonable solution to use [[no_unique_address]] in C++20 (or whenever the compiler supports a similar attribute) but not bother with EBO when the attribute isn't available.

I agree that if ring_span were still on-track-for-standardization, we'd definitely specify the-member-whose-exposition-only-name-is-m_popper as [[no_unique_address]]. :)

dcolascione commented 3 years ago

Doing it that way would make the C++20 version ABI-incompatible with the C++17 version --- doing it with EBO would work in all versions of C++.

martinmoene commented 3 years ago

Thanks for this interesting input... makes me think of nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS :)

martinmoene commented 3 years ago

Comment on commit by @dcolascione:

I don't understand why there's a configuration flag here. The empty base optimization works on all versions of C++, even C++98. Why would anyone want to disable it? Sure, in C++20, we could use the attribute, but the EBO still works in C++20 without it. What's the downside to just using EBO unconditionally everywhere? Seems a lot simpler to me.

The origin for flag nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS lies in ring-span lite being based on proposal p0059. In Readme section In a nutshell I mention my intention to follow the development of that proposal.

Although that proposal appears to not be actively pursued at the moment, I'm somewhat hesitant to deviate from that path right now. I prefer to allow some time and possibly gather some (more) feedback to base my decisions on.

martinmoene commented 3 years ago

Addressed in Release 0.6.0.