RE: [pooma-dev] DynamicArray destroy methods

[James Crotinger]

Perhaps we should use different names for the destroy methods that take iterators. Ordinarily, my preference would be "destroyRange", but Range has a special connotation in Pooma, so perhaps that isn't so good here. Also, for the functions that take a patchID - does the ID have to be local? Is it a local ID? Or global? If local, the perhaps we should go with destroyLocal and destroyGlobal. It just seems that we have too many connotations rolled into one overloade method name.

 

    Jim

 

-----Original Message-----
From: Julian C. Cummings [mailto:address@hidden
Sent: Wednesday, April 25, 2001 9:57 AM
To: address@hidden
Subject: RE: [pooma-dev] DynamicArray destroy methods

Can I get some final comments and closure on this?

I'd like to check in fixes for the problem with overloading

destroy(), and the incorrect return type of non-const

operator() in IteratorPairDomain.  For the first problem,

there seem to be three solutions: pass tags by const ref,

pass destroy methods as an enum type rather than tag

classes, or specialize destroy() to each of the two destroy

methods that we provide.  The first solution is kind of a

tricky hack, but it does work.  The third one is less elegant

but certainly works as well.  Either of these seems OK to me.

 

I'd like to make some decisions on these little items, so I

can check in the fixes and move on.

 

Thanks, Julian C.

 

-----Original Message-----
From: James Crotinger [mailto:address@hidden
Sent: Monday, April 23, 2001 2:21 PM
To: 'address@hidden'
Subject: RE: [pooma-dev] DynamicArray destroy methods

Mark or Allan, any comment on these issues? I'm mostly just following instinct based on what I see in the STL, where iterators, tags, functors, etc., seem to always be passed by value, and only the container's value's are passed by reference. Passing by reference has to incur some overhead for small objects, but perhaps it isn't important (you are, after all, passing a pointer on the stack and not the value, so there has to be at least one dereference in the function body - perhaps the compiler can optimize other derefs away, but only if it doesn't run out of registers for other things). However, I definitely don't understand the second problem Julian's having. The fundamental source of the problem is that we have Domain and Method (tag) template parameters and so we have the following destroy overloads:

 

template <class Dom, class Meth)   destroy(const Dom &, const Meth&);

 

template <class Dom>               destroy(const Dom &);

 

template <class Iter, class Meth)  destroy(Iter begin, Iter end, const Meth &);

 

template <class Iter>              destroy(Iter begin, Iter end);

 

template <class Dom, class Meth)   destroy(const Dom &, int, const Meth &);

 

template <class Dom>               destroy(const Dom &, int);

 

template <class Iter, class Meth>  destroy(Iter, Iter, int, const Meth &);

 

template <class Iter>              destroy(Iter, Iter, int);

 

(We do want to pass Domains by reference here since they are not necessarily small objects. I would tend to pass everything else by value, though, so I'd also change "const Meth &" to just "Meth". But I don't THINK that significantly simplifies the complexity.)

 

Now that I see all of these, I do feel like this is a recipe for disaster. Julian "fixed" it by passing everything by reference, but this seems like a hack. If having both

 

  destroy(const Dom &,const Meth &)

 

and

 

  destroy(Iter,Iter)

 

is causing an ambiguity with:

 

  int *p = ...;

  a.destroy(p, p+3)

 

I'm guessing this is just telling us that we're in template hell. 8-) This looks to me like it should work with the above, but perhaps we've just tried to get too fancy.

 

We could fix the problem by passing an enum instead of a tag, or by explicitly overloading on Shiftup and BackFill, the only two types that are valid to pass as the Meth parameter right now.

 

Thoughts?

 

    Jim

 

 -----Original Message-----
From: Julian C. Cummings [mailto:address@hidden
Sent: Monday, April 23, 2001 2:20 PM
To: James Crotinger; Pooma Developers
Subject: RE: [pooma-dev] DynamicArray destroy methods

Hi Jim,

 

I'm pretty sure your performance concerns are unfounded.

Accessing object methods via reference should never incur

a performance penalty, just as using the dot operator and

the arrow operator should be equivalent.  If this were not the

case, almost all C++ code ever written would have problems.

There is a performance penalty when you pass a heavyweight

object by value, of course.

 

In any case, I did a quick test of your idea.  It can be made

to work, but is slightly more annoying in actual use.  In order

for the pass-by-value to match when passing an iterator pair,

both of the arguments must be local objects that can be copied.

Thus, passing something like (dptr,dptr+3), where dptr is an

int*, once again triggers the ambiguity error.  The compiler thinks

you have passed an int* & and an int*, and these two arguments

are not exactly the same.  So you have to declare a second

int* object and set it equal to dptr+3.  With const references,

this little annoyance is gone.

 

Julian C.

 

P.S.  I stumbled over another potential bug, this time in the

IteratorPairDomain class.  The non-const version of operator()

returns an Element_t&.  This is incorrect when Iter is a const T*.

I think we need to add a typedef for ElementRef_t, which would

be set to std::iterator_traits<Iter>::reference, and return that here.

 

-----Original Message-----
From: James Crotinger [mailto:address@hidden
Sent: Monday, April 23, 2001 12:20 PM
To: 'address@hidden'; Pooma Developers
Subject: RE: [pooma-dev] DynamicArray destroy methods

I tend not to like this idea. Won't passing references result in dereferencing every time you use the object in question? With iterators and ints I would think this could potentially impact performance.

What about going the other way and passing the Destroy tag by value? We might need to write a dummy copy constructor for the tag, but that's not so hard.

        Jim

> -----Original Message-----
> From: Julian C. Cummings [mailto:address@hidden]
> Sent: Monday, April 23, 2001 12:59 PM
> To: Pooma Developers
> Subject: RE: [pooma-dev] DynamicArray destroy methods
>
>
> Never mind!  I just figured out the problem
> and the solution.  The reason for the overload
> ambiguities is that we were defining the
> destroy() functions to take domain and destroy
> method arguments by const reference but take
> iterators and patch IDs by value.  So the
> overloads did not match the function calls in
> exactly the same way, and I guess const int& is
> preferred to int when you pass a literal int
> like "6".  Also, you have to be careful about
> passing with an int array like killList because
> it is not treated quite the same as &killList[0]
> when it comes to argument matching.
>
> Anyhow, once I changed the iterator and patchID
> arguments to be passed by const reference, all
> the function overloads matched as expected.  I'd
> like to check in these fixes to DynamicArray.h
> along with the minor fixes I mentioned in my
> original e-mail on this, barring any objections.
> I am attaching the diff's for DynamicArray.h
> Sorry for the volley of e-mails on this.
>
> Julian C.
>
>
> > -----Original Message-----
> > From: Julian C. Cummings [mailto:address@hidden]
> > Sent: Monday, April 23, 2001 11:19 AM
> > To: Pooma Developers
> > Subject: RE: [pooma-dev] DynamicArray destroy methods
> >
> >
> > One way out of the function overloading
> > quandary I noted below would be to call
> > out versions of destroy() for each of the
> > two existing destroy methods, BackFill
> > and ShiftUp, rather than use a template
> > parameter.  We are unlikely to add more
> > of these, and if we do, they have to be
> > added to the list in DynamicEvents anyway.
> > Should we take this approach?
> >
> > Julian C.
> >
> >
> > > -----Original Message-----
> > > From: Julian C. Cummings [mailto:address@hidden]
> > > Sent: Monday, April 23, 2001 11:08 AM
> > > To: James Crotinger
> > > Cc: Pooma Developers
> > > Subject: [pooma-dev] DynamicArray destroy methods
> > >
> > >
> > > Jim,
> > >
> > > I was going to add new destroy() functions
> > > to Particles corresponding to the new
> > > IteratorPairDomain-based functions you
> > > added to DynamicArray.  I noticed three minor
> > > glitches in DynamicArray.h that I was going
> > > to fix, if you don't object.  One is on
> > > line 486.  We don't need to provide the
> > > BackFill() argument here because dynamic
> > > engines already provide a destroy() function
> > > that takes just a Domain argument.  (No harm
> > > done here though, and I think this glitch
> > > was here prior to your recent changes.)  The
> > > next item is on line 501.  You left out the
> > > Pooma:: qualifier on the IteratorPairDomain
> > > type.  It should be there for consistency,
> > > although there is probably a "using namespace
> > > Pooma;" statement somewhere that is making
> > > this work as written.  The last problem is on
> > > line 542.  I think you do have to provide the
> > > BackFill() argument here because MPEngine only
> > > provides three versions of destroy(): domain,
> > > domain and patchID, or domain, patchID and method.
> > >
> > > I was trying to check out this third item when
> > > I ran into another problem.  Perhaps others can
> > > chime in on this.  I modified dynamic_array_test5.cpp
> > > to call destroy() without specifying a method.  So
> > > I made calls with arguments (int*,int*) and with
> > > (vector<int>::iterator,vector<int>::iterator,int).
> > > The first call should invoke destroy() with Iter
> > > equal to int* (DynamicArray.h, line 498), but gcc
> > > says the overload is ambiguous with the version on
> > > line 477 also being plausible.  I thought matching
> > > one template parameter always beats matching two.
> > > With my second call, I was trying to invoke the
> > > destroy() function on line 538 of DynamicArray.h,
> > > but gcc thinks the version on line 491 is a good
> > > match also.  Again I am surprised because PatchID_t
> > > is defined as int, and that exactly matches the
> > > type of my third argument.  What's the deal?
> > >
> > > Julian C.
> > >
> > >
> >
>