orbitcpp-list

[orbitcpp-list] questions

[Andreas K. <ak...@ix...>]

Hi Phil,

got some questions regarding the runtime:

* why the extra op_marshaller method in the skels? why not everything in
one skel wrapper?
* are you sure the environment singleton does not cause reentrancy
problems (e.g. a calls b and b calls a back before returning) apart from
being not thread-safe (aka: is creating the env on the stack much more
inefficient?)
* what specifically was the problem with keeping servants on the stack?
(it worked for o2cpp)
* shouldn't we prefix every non-spec method in public inheritable
classes by something like _orbitcpp to reduce the risk of name clashes?
* shouldn't we reindent this with indent-namespaces on? (if yes, go
ahead, see .astylerc from other mail)

* do you still have a copy of o2cpp handy? if yes, have a look at the
following:
- lang_c.cc: duplicateGuarded,releaseGuarded. prevents missing
exceptions.
- o2cpp_smartptr.hh: what about taking this implementation instead of
all the "nicked from mico&omniorb" stuff? (pro: wchar support, nice and
consistent; con: breaks existing code)
- o2cpp_exception.hh: looks more complete than yours

that's it for tonight. i'm getting some sleep.

cya
andy

Re: [orbitcpp-list] questions

[Phil d. <ph...@us...>]

Andreas Kloeckner wrote:
> 
> Hi Phil,
> 
> got some questions regarding the runtime:
> 
> * why the extra op_marshaller method in the skels? why not everything in
> one skel wrapper?

It's a multiple inheritance issue:
For each call the skeleton operation needs to cast the void* pointer
into the appropriate C++ skeleton type. With multiple inheritence
(virtual inheritance) C++ relies on the cast to do the appropriate
vtable thunk. This means that you need a function per interface per
operation to ensure that the void* c++ servant pointer gets casted to
the right C++ servant type (you cant share them).
The marshalling code for inherited operations is common across all the
interfaces, and so gets factored out into a single op_marshaller
function.

If you grab a copy of ORBit-C++ and compile the diamond-inheritance test
and look at the skels file, you will see how this works. (mail me if you
want me to send you the C file and save you faffing about with
PYTHON_PATHs ;-)

> * are you sure the environment singleton does not cause reentrancy
> problems (e.g. a calls b and b calls a back before returning) apart from
> being not thread-safe (aka: is creating the env on the stack much more
> inefficient?)

You're righ, it will cause reentrancy problems and it isn't thread safe.
I wrapped it in an accessor method in order to defer the decision on how
to create/manage environments until later. e.g. maybe some pooling will
yield the best performance/thread safety compromise.
Creating and initialising envs on the stack may be more efficient as
well.

> * what specifically was the problem with keeping servants on the stack?
> (it worked for o2cpp)

I can't remember the specific reason. The symptom was that when the c++
implementation object was created on the stack, we got a segfault.
Creating the C servant on the heap fixed this.
If we can get away with creating c servants on the stack then this is
obviously preferable from a performance standpoint.

> * shouldn't we prefix every non-spec method in public inheritable
> classes by something like _orbitcpp to reduce the risk of name clashes?

Sounds reasonable.

> * shouldn't we reindent this with indent-namespaces on? (if yes, go
> ahead, see .astylerc from other mail)

Okay. I don't mind either way.

Cheers,

Phil.

Re: [orbitcpp-list] questions

[Andreas K. <ak...@ix...>]

> > * why the extra op_marshaller method in the skels? why not everything in
> > one skel wrapper?
> 
> It's a multiple inheritance issue:
> For each call the skeleton operation needs to cast the void* pointer
> into the appropriate C++ skeleton type. With multiple inheritence
> (virtual inheritance) C++ relies on the cast to do the appropriate
> vtable thunk. This means that you need a function per interface per
> operation to ensure that the void* c++ servant pointer gets casted to
> the right C++ servant type (you cant share them).
> The marshalling code for inherited operations is common across all the
> interfaces, and so gets factored out into a single op_marshaller
> function.
> 
> If you grab a copy of ORBit-C++ and compile the diamond-inheritance test
> and look at the skels file, you will see how this works. (mail me if you
> want me to send you the C file and save you faffing about with
> PYTHON_PATHs ;-)

Got that. In o2cpp, I chose to generate all the marshalling code for
every inheriting interface, leaving a "*** FIXME we might save code
here" (it's still there - see pass_stubs.cc :). Of course, this was a
kludge. I also had the two-method solution in mind (yet didn't remember
it yesterday), but i wondered whether there was some more efficient way
than to re-pass all those parameters. i thought of doing it by messing
with the servant struct in some tricky way, but obviously this couldn't
survive diamond-shaped inheritance. something else i thought of was
having sort of a widening function for every interface, pointed to by
the servant struct, but this is neither faster nor more elegant. can you
think of a better way? if not, the two-method thing is the way to go.

one improvement for the two-method thing just came to my mind: keep the
*_marshaller as well as the *_callback (i would call it *_caster for
clarity) static methods and thus be able to put the *_marshaller into
the epv. this saves an extra call in a non-inheritance setting.

> > * are you sure the environment singleton does not cause reentrancy
> > problems (e.g. a calls b and b calls a back before returning) apart from
> > being not thread-safe (aka: is creating the env on the stack much more
> > inefficient?)
> 
> You're righ, it will cause reentrancy problems and it isn't thread safe.
> I wrapped it in an accessor method in order to defer the decision on how
> to create/manage environments until later. e.g. maybe some pooling will
> yield the best performance/thread safety compromise.
> Creating and initialising envs on the stack may be more efficient as
> well.

Have a look at the cEnvironment thing from old o2cpp
(o2cpp_exception.hh) and tell me what you think of it.

cu
andy

Re: [orbitcpp-list] questions

[Phil d. <ph...@us...>]

Andreas Kloeckner wrote:
> 
> Hi Phil,
> 
> 
> * do you still have a copy of o2cpp handy? if yes, have a look at the
> following:

I've got o2cpp-0.10

> - lang_c.cc: duplicateGuarded,releaseGuarded. prevents missing
> exceptions.

Yep. That's good.

> - o2cpp_smartptr.hh: what about taking this implementation instead of
> all the "nicked from mico&omniorb" stuff? (pro: wchar support, nice and
> consistent; con: breaks existing code)

<glances through code>
Is it completely spec compliant? Tested? (I spent very little time
looking at the mico code - I nicked it because it was already fairly
complete and tested)

It looks good to me, I have no problem with replacing the existing code
with this (s/o2cpp/orbitcpp/g etc..) in an orbitcpp_smartptr.hh file or
something.
How does this break existing code? What stops us from just bolting this
in?

> - o2cpp_exception.hh: looks more complete than yours

Yep, and I like the cEnvironment class. Feel free to replace the
exception code with this.

Cheers,

Phil.