Discussion:
[omniORB] Corba union waste of space?
Michael Teske
2008-12-04 23:46:35 UTC
Permalink
Hi,


please conside this file testunion.idl :

--- snip ---


module test {
enum DataType {
StringType,
CharType,
CardinalType,
IntegerType,
PriceType,
VolumeType,
QuantityType,
DateType,
TimeType,
StringArrayType
};

typedef string _String;
typedef char _Char;
typedef unsigned long _Cardinal;
typedef long _Integer;
typedef string _Price;
typedef string _Volume;
typedef string _Quantity;
typedef string _Date;
typedef string _Time;
typedef sequence<string> _StringArray;

union TheTestUnion switch (DataType) {
case StringType:
_String StringData;
case CharType:
_Char CharData;
case CardinalType:
_Cardinal CardinalData;
case IntegerType:
_Integer IntegerData;
case PriceType:
_Price PriceData;
case VolumeType:
_Volume VolumeData;
case QuantityType:
_Quantity QuantityData;
case DateType:
_Date DateData;
case TimeType:
_Time TimeData;
case StringArrayType:
_StringArray StringArrayData;
};
};

--- snip ---

When I compile it with omniidl -bcxx, it genedates the following C++ code in the
private: section of "class TheTestUnion" in testunion.hh:

private:
DataType _pd__d;
_CORBA_Boolean _pd__default;
_CORBA_Boolean _pd__initialised;

union {
Char _pd_CharData;

Cardinal _pd_CardinalData;

Integer _pd_IntegerData;


};


::CORBA::String_member _pd_StringData;

::CORBA::String_member _pd_PriceData;

::CORBA::String_member _pd_VolumeData;

::CORBA::String_member _pd_QuantityData;

::CORBA::String_member _pd_DateData;

::CORBA::String_member _pd_TimeData;

StringArray _pd_StringArrayData;




This seems like a bug (waste of space) to me or is there any reason that the
string members do not go into the union? Our real-life used union has even more
types... As I mentioned in an earlier post, we're porting our system from orbacus,
which uses pointers here so they fit into the C++ union data type (I suppose the
complex types are not allowed in a C++union?).

Greetings,
Michael
Bruce Visscher
2008-12-05 00:27:45 UTC
Permalink
Post by Michael Teske
This seems like a bug (waste of space) to me or is there any reason that the
string members do not go into the union?
(I suppose the
complex types are not allowed in a C++union?).
[...] orbacus [...] uses pointers here so they fit into the C++ union data type
For some reason I had thought that the space wasting implementation
was somehow required by the standard but maybe not.

In C++ I find that I only occasionally wish I had some kind of
discriminated union type ("plain ol;" unions are of course odious).
Maybe that is because I can use polymorphism or generic programming as
an alternative (or even casting through void*). But since IDL doesn't
have classes (an interface isn't quite the same thing) it does seems
to come up more often. However, I usually don't worry about wasting
the space. If you think about it, your example isn't as bad as it
might seem at first. It only "wastes" instances of the class
String_member which are only going to have pointers and (maybe) ints
in them. The unbounded string data is on the heap.

I suppose if IONA doesn't have a patent on this (it is obvious so it
shouldn't) maybe someone could implement this if they wanted.
Teemu Torma
2008-12-05 03:14:00 UTC
Permalink
Post by Bruce Visscher
In C++ I find that I only occasionally wish I had some kind of
discriminated union type.
Boost.Variant is pretty much that. Internally it has a space for the largest
type and uses placement new to construct the active member there.

Teemu
Michael Teske
2008-12-05 15:08:23 UTC
Permalink
Hi!
Post by Bruce Visscher
Post by Michael Teske
This seems like a bug (waste of space) to me or is there any reason that the
string members do not go into the union?
(I suppose the
complex types are not allowed in a C++union?).
Yes, I fear so...
Post by Bruce Visscher
Post by Michael Teske
[...] orbacus [...] uses pointers here so they fit into the C++ union data type
For some reason I had thought that the space wasting implementation
was somehow required by the standard but maybe not.
In C++ I find that I only occasionally wish I had some kind of
discriminated union type ("plain ol;" unions are of course odious).
Maybe that is because I can use polymorphism or generic programming as
an alternative (or even casting through void*). But since IDL doesn't
have classes (an interface isn't quite the same thing) it does seems
to come up more often. However, I usually don't worry about wasting
the space. If you think about it, your example isn't as bad as it
might seem at first. It only "wastes" instances of the class
String_member which are only going to have pointers and (maybe) ints
in them. The unbounded string data is on the heap.
In our case (we have more datatypes than in my example) it's 12 bytes (orbacus)
versus 172 bytes (omniorb), for an empty object.
But anyway the question is if it was such a great idea to use Corba datatypes for
internal storage.
Post by Bruce Visscher
I suppose if IONA doesn't have a patent on this (it is obvious so it
shouldn't) maybe someone could implement this if they wanted.
I'd like to try, I suppose it's omniidl where this is done, somewhere in the
python code?

Greetings,
Michael
Duncan Grisby
2008-12-08 17:18:57 UTC
Permalink
On Thursday 4 December, Michael Teske wrote:

[...]
Post by Michael Teske
This seems like a bug (waste of space) to me or is there any reason
that the string members do not go into the union? Our real-life used
union has even more types... As I mentioned in an earlier post, we're
porting our system from orbacus, which uses pointers here so they fit
into the C++ union data type (I suppose the complex types are not
allowed in a C++union?).
The String_members cannot go in the union, because you can't put such
things in a C++ union. It certainly would be possible to store pointers
to String_members (and other complex types) instead, and they could go
in the union. I'd welcome a patch that did that.

The place to start attacking it is to look in
src/lib/omniORB/omniidl_be/cxx/header/template.py which defines the
templates that go in the C++ headers. From there, search the code for
the places that use the templates. I'm afraid that some of the C++
back-end code is rather ugly...

Cheers,

Duncan.
--
-- Duncan Grisby --
-- ***@grisby.org --
-- http://www.grisby.org --
Bruce Visscher
2008-12-09 19:19:53 UTC
Permalink
Post by Duncan Grisby
[...]
Post by Michael Teske
This seems like a bug (waste of space) to me or is there any reason
that the string members do not go into the union? Our real-life used
union has even more types... As I mentioned in an earlier post, we're
porting our system from orbacus, which uses pointers here so they fit
into the C++ union data type (I suppose the complex types are not
allowed in a C++union?).
The String_members cannot go in the union, because you can't put such
things in a C++ union. It certainly would be possible to store pointers
to String_members (and other complex types) instead, and they could go
in the union. I'd welcome a patch that did that.
The place to start attacking it is to look in
src/lib/omniORB/omniidl_be/cxx/header/template.py which defines the
templates that go in the C++ headers. From there, search the code for
the places that use the templates. I'm afraid that some of the C++
back-end code is rather ugly...
Thanks Duncan.

Michael, one thing I would suggest (inspired by Teemu's post) is that
if you are going to spend the time doing this you consider an
implementation that utilizes placement new and explicit calls to the
destructor (as boost variant apparently does).

A brief outline would be (using String_member as an example) declare a
char array[sizeof(String_member)] as a union member. Also declare
(but do not use) something with maximal alignment requirements
(void*'s and/or long long are good choices you could actually use
both). This would ensure that the storage will be aligned on a 32 or
64 bit boundary. Then use placement new to initialize the storage and
an explicit call to the destructor "deinitialize" the storage. This
would avoid an extra memory allocation. Use
reinterpret_cast<String_member*> on the char pointer in the
String_member accessor.

Just a thought,
Michael Teske
2008-12-10 05:26:18 UTC
Permalink
Post by Bruce Visscher
Post by Duncan Grisby
[...]
Post by Michael Teske
This seems like a bug (waste of space) to me or is there any reason
that the string members do not go into the union? Our real-life used
union has even more types... As I mentioned in an earlier post, we're
porting our system from orbacus, which uses pointers here so they fit
into the C++ union data type (I suppose the complex types are not
allowed in a C++union?).
The String_members cannot go in the union, because you can't put such
things in a C++ union. It certainly would be possible to store pointers
to String_members (and other complex types) instead, and they could go
in the union. I'd welcome a patch that did that.
The place to start attacking it is to look in
src/lib/omniORB/omniidl_be/cxx/header/template.py which defines the
templates that go in the C++ headers. From there, search the code for
the places that use the templates. I'm afraid that some of the C++
back-end code is rather ugly...
Thanks Duncan.
Michael, one thing I would suggest (inspired by Teemu's post) is that
if you are going to spend the time doing this you consider an
implementation that utilizes placement new and explicit calls to the
destructor (as boost variant apparently does).
A brief outline would be (using String_member as an example) declare a
char array[sizeof(String_member)] as a union member. Also declare
(but do not use) something with maximal alignment requirements
(void*'s and/or long long are good choices you could actually use
both). This would ensure that the storage will be aligned on a 32 or
64 bit boundary. Then use placement new to initialize the storage and
an explicit call to the destructor "deinitialize" the storage. This
would avoid an extra memory allocation. Use
reinterpret_cast<String_member*> on the char pointer in the
String_member accessor.
Just a thought,
Hi Bruce, hi Duncan,

this sounds resonable, so I think I'll do it this way, but it might take
a while since I go on vacation next week.

Greetings,
Michael
Michael Teske
2009-01-06 21:07:06 UTC
Permalink
Post by Bruce Visscher
Post by Duncan Grisby
[...]
Post by Michael Teske
This seems like a bug (waste of space) to me or is there any reason
that the string members do not go into the union? Our real-life used
union has even more types... As I mentioned in an earlier post, we're
porting our system from orbacus, which uses pointers here so they fit
into the C++ union data type (I suppose the complex types are not
allowed in a C++union?).
The String_members cannot go in the union, because you can't put such
things in a C++ union. It certainly would be possible to store pointers
to String_members (and other complex types) instead, and they could go
in the union. I'd welcome a patch that did that.
The place to start attacking it is to look in
src/lib/omniORB/omniidl_be/cxx/header/template.py which defines the
templates that go in the C++ headers. From there, search the code for
the places that use the templates. I'm afraid that some of the C++
back-end code is rather ugly...
Thanks Duncan.
Michael, one thing I would suggest (inspired by Teemu's post) is that
if you are going to spend the time doing this you consider an
implementation that utilizes placement new and explicit calls to the
destructor (as boost variant apparently does).
A brief outline would be (using String_member as an example) declare a
char array[sizeof(String_member)] as a union member. Also declare
(but do not use) something with maximal alignment requirements
(void*'s and/or long long are good choices you could actually use
both). This would ensure that the storage will be aligned on a 32 or
64 bit boundary. Then use placement new to initialize the storage and
an explicit call to the destructor "deinitialize" the storage. This
would avoid an extra memory allocation. Use
reinterpret_cast<String_member*> on the char pointer in the
String_member accessor.
Hi Bruce, hi Duncan,

finally I found the time to do it. Unfortunately I ran into 2 problems, one can be
solved with an ugly trick, but for the other one I can't find an easy solution:

1. (difficult): In skutil.py/unmarshall I need to find out if a string (or other
types, to be done later) is a member of a union or not (in the first case I have
to cast the char array to ::CORBA::String_member * and dereference it, in the
other case I leave the code as it is), but I cannot find a way to get this info.

2. (ugly trick): In order for placement new to work 100% correct, I need to call
destructors explicitely. Unfortunately I only have the type name
:CORBA::String_member and need to call ~_CORBA_String_member(), because it's a
typedef. My ugly hack is to replace ::CORBA:: with _CORBA_, but I really do not
like it, as it will probably break with other types.


Any ideas?

Greetings,
Michael
Michael Teske
2009-01-07 02:58:34 UTC
Permalink
Post by Bailey, Kendall
Post by Michael Teske
2. (ugly trick): In order for placement new to work 100% correct, I need to call
destructors explicitely. Unfortunately I only have the type name
:CORBA::String_member and need to call ~_CORBA_String_member(), because it's a
typedef. My ugly hack is to replace ::CORBA:: with _CORBA_, but I really do not
like it, as it will probably break with other types.
Any ideas?
If I understand the issue, it seems a simple template function would do the trick. Something like this?
template<typename T>
destroy_union_member( T* p )
{
p->~T();
}
Of course, thank you very much!

Greetings,
Michael

Michael Teske
2009-01-06 21:46:21 UTC
Permalink
Michael Teske wrote:
[...]
Post by Michael Teske
Hi Bruce, hi Duncan,
finally I found the time to do it. Unfortunately I ran into 2 problems, one can be
1. (difficult): In skutil.py/unmarshall I need to find out if a string (or other
types, to be done later) is a member of a union or not (in the first case I have
to cast the char array to ::CORBA::String_member * and dereference it, in the
other case I leave the code as it is), but I cannot find a way to get this info.
Ok, I found it for 1. Wasn't so difficult after all. That leaves just the ugliness
of 2.

Greetings,
Michael
Post by Michael Teske
2. (ugly trick): In order for placement new to work 100% correct, I need to call
destructors explicitely. Unfortunately I only have the type name
:CORBA::String_member and need to call ~_CORBA_String_member(), because it's a
typedef. My ugly hack is to replace ::CORBA:: with _CORBA_, but I really do not
like it, as it will probably break with other types.
Any ideas?
Greetings,
Michael
_______________________________________________
omniORB-list mailing list
http://www.omniorb-support.com/mailman/listinfo/omniorb-list
Bailey, Kendall
2009-01-06 22:48:16 UTC
Permalink
Post by Michael Teske
2. (ugly trick): In order for placement new to work 100% correct, I need to call
destructors explicitely. Unfortunately I only have the type name
:CORBA::String_member and need to call ~_CORBA_String_member(), because it's a
typedef. My ugly hack is to replace ::CORBA:: with _CORBA_, but I really do not
like it, as it will probably break with other types.
Any ideas?
If I understand the issue, it seems a simple template function would do the trick. Something like this?

template<typename T>
destroy_union_member( T* p )
{
p->~T();
}


----
Kendall Bailey
Loading...