Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Empty base optimization for m_popper #23

Closed
dcolascione opened this issue Jun 27, 2021 · 5 comments
Closed

Empty base optimization for m_popper #23

dcolascione opened this issue Jun 27, 2021 · 5 comments

Comments

@dcolascione
Copy link

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
Copy link

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
Copy link
Author

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
Copy link
Owner

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

martinmoene added a commit that referenced this issue Jul 12, 2021
@martinmoene
Prevent stateless poppers to take up space (#23, thanks @dcolascione, @…
b35eef8 
…Quuxplusone)

- Add C++20 attribute [[no_unique_address]]
- Add configuration macro nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS.

vBox|Manjaro:
- gcc (GCC) 11.1.0, -std=C++17, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=0: sizeof(ring_span): 40
- gcc (GCC) 11.1.0, -std=C++17, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=1: sizeof(ring_span): 32
- gcc (GCC) 11.1.0, -std=C++20, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=0: sizeof(ring_span): 32
- gcc (GCC) 11.1.0, -std=C++20, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=1: sizeof(ring_span): 32

Windows 10:
- MSVC 19.28 (VS2019), -std:c++latest, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=0: sizeof(ring_span): 20
- MSVC 19.28 (VS2019), -std:c++latest, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=1: sizeof(ring_span): 16
martinmoene added a commit that referenced this issue Jul 12, 2021
@martinmoene
Prevent stateless poppers to take up space (#23, thanks @dcolascione, @…
8ec1822 
…Quuxplusone)

- Add C++20 attribute [[no_unique_address]]
- Add configuration macro nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS.

vBox|Manjaro:
- gcc (GCC) 11.1.0, -std=C++17, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=0: sizeof(ring_span): 40
- gcc (GCC) 11.1.0, -std=C++17, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=1: sizeof(ring_span): 32
- gcc (GCC) 11.1.0, -std=C++20, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=0: sizeof(ring_span): 32
- gcc (GCC) 11.1.0, -std=C++20, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=1: sizeof(ring_span): 32

Windows 10:
- MSVC 19.28 (VS2019), -std:c++latest, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=0: sizeof(ring_span): 20
- MSVC 19.28 (VS2019), -std:c++latest, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=1: sizeof(ring_span): 16
@martinmoene
Copy link
Owner

martinmoene commented Jul 12, 2021 •
edited
Loading

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 added a commit that referenced this issue Jul 17, 2021
@martinmoene
Prevent stateless poppers to take up space (#23, thanks @dcolascione, @…
ce88b9d 
…Quuxplusone)

- Add C++20 attribute [[no_unique_address]]
- Add configuration macro nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS.

vBox|Manjaro:
- gcc (GCC) 11.1.0, -std=C++17, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=0: sizeof(ring_span): 40
- gcc (GCC) 11.1.0, -std=C++17, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=1: sizeof(ring_span): 32
- gcc (GCC) 11.1.0, -std=C++20, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=0: sizeof(ring_span): 32
- gcc (GCC) 11.1.0, -std=C++20, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=1: sizeof(ring_span): 32

Windows 10:
- MSVC 19.28 (VS2019), -std:c++latest, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=0: sizeof(ring_span): 20
- MSVC 19.28 (VS2019), -std:c++latest, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=1: sizeof(ring_span): 16
martinmoene added a commit that referenced this issue Jul 17, 2021
@martinmoene
Add [[no_unique_address]] to m_popper (#23, thanks @dcolascione, @Quu…
f0851af 
…xplusone)

Add macros nsrs_HAVE_NO_UNIQUE_ADDRESS and nsrs_NO_UNIQUE_ADDRESS
martinmoene added a commit that referenced this issue Jul 17, 2021
@martinmoene
Prevent stateless poppers to take up space (#23, thanks @dcolascione, @…
a050278 
…Quuxplusone)

- Add C++20 attribute [[no_unique_address]]
- Add configuration macro nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS.

vBox|Manjaro:
- gcc (GCC) 11.1.0, -std=C++17, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=0: sizeof(ring_span): 40
- gcc (GCC) 11.1.0, -std=C++17, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=1: sizeof(ring_span): 32
- gcc (GCC) 11.1.0, -std=C++20, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=0: sizeof(ring_span): 32
- gcc (GCC) 11.1.0, -std=C++20, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=1: sizeof(ring_span): 32

Windows 10:
- MSVC 19.28 (VS2019), -std:c++latest, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=0: sizeof(ring_span): 20
- MSVC 19.28 (VS2019), -std:c++latest, nsrs_CONFIG_POPPER_EMPTY_BASE_CLASS=1: sizeof(ring_span): 16
@martinmoene
Copy link
Owner

Addressed in Release 0.6.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@dcolascione @Quuxplusone @martinmoene