dolfin team mailing list archive

[Johan Hake <johan.hake@xxxxxxxxx>]

On Monday 08 February 2010 01:15:12 Anders Logg wrote:
> On Sun, Feb 07, 2010 at 11:25:28PM +0000, Garth N. Wells wrote:
> > Anders Logg wrote:
> > > On Sun, Feb 07, 2010 at 10:58:44PM +0000, Garth N. Wells wrote:
> > >> Anders Logg wrote:
> > >>> This looks a like a non-optimal solution. Are you returning the mesh
> > >>> by value? That will lead to creation of a temporary and copying of
> > >>> the entire refined mesh.
> > >>
> > >> So they used say ;). Apparently modern compilers are all clever enough
> > >> not to.
> > >>
> > >> Garth
> > >
> > > Is that really so? Do you have any good references that explain this
> > > in detail?
> >
> >   http://en.wikipedia.org/wiki/Return_value_optimization
> >
> > and from 'man gcc'
> >
> > -fno-elide-constructors
> >   The C++ standard allows an implementation to omit creating a temporary
> > which is only used to initialize another object of the same type.
> > Specifying this option disables that optimization, and forces G++ to
> > call the copy constructor in all cases.
> 
> ok, seems correct. I tried the following piece of code with some
> debugging added to the copy constructor and assignment operator in
> class Mesh:
> 
>   Mesh refined_mesh_1 = UniformMeshRefinement::refine(mesh);
> 
>   Mesh refined_mesh_2;
>   refined_mesh_2 = UniformMeshRefinement::refine(mesh);
> 
> The first refinement does not lead to any call to either of the copy
> constructor or assignment operator.
> 
> The second call leads to one call to the assignment operator.
> 
> This is expected but means we need to be careful with how the
> refinement is called (on the same line as initialization).
> 
> Should we have a look and see if there are other places where we could
> return by value?

The operator* and operator+ were explicitly not included because they needed 
to be implemented as a return by value method. Now they can be added in the 
same way as the Python equivalents I think.

Johan

> We also need to add refine.h/refine.cpp and make refine() a free
> function as discussed before to avoid dependending on adaptivity
> library from the mesh class.
> 
> --
> Anders
> 
> > Garth
> >
> > This could potentially lead to many changes in DOLFIN where
> >
> > > we could return by value rather than by reference with the prime
> > > example being
> > >
> > >   A = assemble(a);
> > >
> > > But does it really handle non-copying of a wrapped PETSc data
> > > structure?
> > >
> > >>> What if we have
> > >>>
> > >>>   FooRefinement::refine(old_mesh, new_mesh)
> > >>>
> > >>> or similar?
> > >>>
> > >>>> ------------------------------------------------------------
> > >>>> revno: 4461
> > >>>> committer: Garth N. Wells <gnw20@xxxxxxxxx>
> > >>>> branch nick: dolfin-nompi
> > >>>> timestamp: Sun 2010-02-07 21:53:42 +0000
> > >>>> message:
> > >>>>   Work on mesh refinement interface.
> > >>>>
> > >>>>   Do not overwrite old mesh inside refinement functions belonging to
> > >>>> static classes. Need to keep old mesh to peform update in adapative
> > >>>> methods, so previously the mesh had to be copied before calling
> > >>>> refine(...). modified:
>