Hi,
On Fri, 2007-10-26 at 19:36 -0500, Carsten Neumann wrote:
> Hi Gerrit,
>
> Gerrit Voss wrote:
> > On Thu, 2007-10-25 at 08:54 -0500, Carsten Neumann wrote:
> >> Gerrit Voss wrote:
> >>>
> > Not quite, the goal was to simplify things, not per se getting rid of
> > something. So IMHO if 'getting rid' of something does not work towards
> > the goal something should be changed.
>
> yes, simpler is THE thing all this ref counting pointers in fields does
> not achieve. Unfortunately I see no way to hide the different pointer
> types as long as there are functions that return a pointer or reference
> to the actual field. One can only pretend up to a certain point that a
> std::vector<ReferenceCountPointer<Node,
> InternalRefCountPolicy<FieldContainer>, FieldContainer> > is the same
> thing as a std::vector<FieldContainer *> even after resorting to
> hair-raising stunts involving reinterpret_cast -- which I'm convinced
> will cause some pain by gcc 4.4 at the latest, but that's a different story.
Hmm, but most likely you will keep these casts in a way, the adaptors
cast too. In general I don't have a problem with these if the design
put on stricter requirements than the language would I'm fine with
it.
> >>
> > Hmm, but in order to do this you record, transmit and apply ref count
> > operations to the remote aspect (or has this changed, the code looks
> > still the same ?). As you do it only for a limited set of objects/calls
> > I still would call it an optimisation ;-). Because it should work if you
> > do it for all objects/calls.
>
> No, doing it for internal/weak pointers will at least produce wrong ref
> count values on the remote side: As soon as a pointer field is populated
> on the remote side the respective ref count of the pointed to object is
> adjusted (that's what the pointers stored in the field do), applying the
> adjustments from the transmitted changelist on top of this screws up the
> values.
hmm, why ?, you just add a fixed offset on the remote site. What happens
right now it that it is a mix. We use the changelist ref counting to
increase the ref count on the remote side and the internal ref counting
to decrease it. To make this work we filter certain subrefs from being
applied. Adding the local ref counting to the remote side would just
mean we remove the filter and it should work.
> >> I guess it could be hidden a bit
> >> more by having the respective MFields return proxy objects instead of
> >> references from operator[] - for MF<FOO>ParentPtr this is of course a
> >> must, if they are exposed for write access, it's on my TODO.
> >
> > But than why not put the ref counting into fields instead of pointer
> > classes. This way we get this part out of the normal class code (which
> > AFAIK is the goal). And if you want to put the parent handling into the
> > fields to you blow them up anyway.
>
> The problem with that are the usual suspects operator[], front(), back()
> which return a reference.
Return a proxy like std::vector<bool> does. Ok, as you don't use
auto_ptr you might pay a small price.
> Anyway, putting the parent handling into the fields would be nice as it
> would allow write access to them, however I'm still thinking about how
> to do it, as I believe I need to know the exact type of the container in
> which the field is contained -- Pointer fields are currently partial
> specializations of MField, which means I can not add an arbitrary
> additional template parameter that provides this information.
Only if you want to store a specific parent. Except for limited cases
like Node I don't see this necessary, you can always convert it on the
way out and sometimes, like for NodeCore, you can have more than one
possible parent class.
> > If you want to force the user to use ref ptrs you can still do so by
> > returning a kind of auto ptrs from everything , the class interface and
> > the fields. This currently is the solution I tend to find the best
> > compromise.
>
> I believe the "getting the user to use ref ptrs" part is better achieved
> by consistently using them in examples and test programs and also
> utilities like the SimpleSceneManager, well basically everywhere except
> maybe the RenderTraversalAction ;)
That was an implicit assumption ;-). I was more meant as a
reinforcement ;-).
> This way we don't have to take any form of performance penalty for
> passing around anything different from a C ptr.
>
> > And it gives you more options to optimise internally without sacrificing
> > a clean user interface.
>
> Do you have thought this through to the end? Could you provide some more
> details, please? (I know it is tedious and time consuming to write these
> things in mails, but I'd really like to find a nicer solution than what
> I'm heading for right now, so please share your thoughts)
Well I would not say 100% but I can see several advantages.
You can treat the different cases as different interfaces
to an identical storage, much like the field adaptors do right now.
So you can easily switch between the interface you return and the
interface you store. Which seem to be similar to the cast hacks
you describe above, but as you have a unified storage I still find
them cleaner (the adaptors cast in a similar way)
If you don't want to do that, you can add an optimised functional
interface (like raw uncounted access), either make it private and only
allow specificfunctions to use it (like sync and the remote aspect
copying), or tag it properly as dangerous, e.g with something like
'WARNING, might melt your core, be sure you know what you are doing'
and still everything as seen by the user, especially the safer interface
for the user, is always the same.
Also one should be able to build it incrementally from the current HEAD
as the base pointer fields are classes on their own.
>
> On the progress report side:
> - the last compile errors I get come from the FileIO library, most seem
> easy to fix by replacing SFNodePtr::iterator with
> Node::ChildrenFieldType::iterator or similar things.
>
> - one thing seems a bit more tricky though: The new osb loader uses the
> following:
>
> if(fieldType.getContentType().isDerivedFrom(
> FieldTraits<FieldContainerPtr>::getType()) == true)
>
> Now, with the whole zoo of pointer types this simple check does not
> quite cut it anymore. Of course I could check
> FieldTraits<FieldContainerInternalRefPtr>,
> FieldTraits<FieldContainerWeakRefPtr> and
> FieldTraits<FieldContainerParentPtr> but that is a bit more verbose ;)
> Should I teach our type system about pointer types or do you have a
> better idea how to do this
Hmm, first thing parents should never be written somewhere, so you don't
need the third case.
For the last 2, ideally from the loader/generic interfaces point of view
it should not make any difference.
Which is a problem with the RefPtr things as you most likely break it
for the get case, well you break it for the weak ptrs too, so we
need a solution anyway.
But still there should be the base edit/get handle class with which you
can work without knowing the actual storage requirements behind.
One change you have to make is that the GetHandle does not return the
field but allows you to iterate over the contents, in a suitable form,
so the writing works.
You probably have to put something in between
{Get|Edit}{S|M}FieldHandle<{S|M}FFieldContainerPtr> and {Get|Edit}Handle
something like {Edit|Get}Handle{S|M}FContainerPtr to cover the generic
cases.
The EditHandle should only be changed in the sense that the parameters
to for example add and set have to cope with the different pointer
types. As it works by calling back into the containing class the actual
storage should not matter, as the interface of the containing container
and the edit handle should be able to deal with exactly the same amount
of parameter variations, or in case you want to limit the generic
interface the interface of the containing class has more variations.
> (I'm not so excited about tentatively casting
> the field handle as is done, e.g. in the GraphOps) ?
Than derive every pointer type from a common (dummy) base class, for
example ContainerPtrType and change the test to
if(fieldType.getContentType().isDerivedFrom(ContainerPtrType) == true)
As said above, what happens after this test should not depend on
concrete ref ptr class stored somewhere else.
kind regards,
gerrit
-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
Opensg-core mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensg-core
