Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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.

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

Union support not optimal in memory usage [13725] #39

Closed
jwillemsen opened this issue Jul 7, 2020 · 5 comments
Closed

Union support not optimal in memory usage [13725] #39

jwillemsen opened this issue Jul 7, 2020 · 5 comments
Labels
enhancement New feature or request

Comments

@jwillemsen
Copy link

The current code generation for IDL unions generates a class with for each case a plain member resulting in memory usage for all types independent of the discriminator, see for example https://github.com/eProsima/Fast-DDS/blob/6fa5e5cc8abcf506e70be203934bd3c522bd3edf/test/xtypes/idl/Types.h#L4693. This leads to a lot of memory usage when larger unions are used. This can be done much more optimized by using an union internally, see for example https://github.com/RemedyIT/idl2cpp11/blob/master/examples/union/testC.h for a possible solution

@MiguelCompany MiguelCompany transferred this issue from eProsima/Fast-DDS Jul 30, 2020
@michaelbgratton
Copy link

michaelbgratton commented Dec 28, 2020

Is this optimization still on the table? I'm interested in this feature. It looks to me that this could be done quite simply using an anonymous union:[https://en.cppreference.com/w/cpp/language/union]

Basically, the emitted code is already a variant (aka discriminating union), in that it has an enum type for the active member, followed by the allowed member types. Simply surrounding these types in union { } should produce the desired effect.

@michaelbgratton
Copy link

To be explicit, surrounding here with union{ } should do the trick.

@jwillemsen
Copy link
Author

jwillemsen commented Jan 1, 2021

It is not as simple when string or structured types are used within the union, at that moment initialization and destruction requires additional code

@michaelbgratton
Copy link

Yes, and std::vector for the sequence too. The easier path would be to use std::variant which provides exactly the discriminating union type needed, but it can be an up-hill battle to get c++17 features accepted.

@JLBuenoLopez JLBuenoLopez changed the title Union support not optimal in memory usage Union support not optimal in memory usage [13725] Feb 1, 2022
@MRicoIE2CS MRicoIE2CS added the enhancement New feature or request label Mar 17, 2023
@MRicoIE2CS
Copy link

I see that this is a proposal for achieving a more optimal behavior regarding memory usage. For that reason, I proceed and convert this issue to a discussion.
Thank you all for the contribution.

@eProsima eProsima locked and limited conversation to collaborators Mar 17, 2023
@MRicoIE2CS MRicoIE2CS converted this issue into discussion #165 Mar 17, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants