[Networkit] Solving the copy vs. swap problem in Cython

Michael Hamann michael.hamann at kit.edu
Mon Sep 1 18:04:17 CEST 2014


Hi,

Am Montag, 1. September 2014, 11:31:28 schrieb Christian Staudt:
> Am 27.08.2014 um 18:19 schrieb Michael Hamann <michael.hamann at kit.edu>:
> > Now after adding the missing "except +" statements, setThis() can be
> > defined as> 
> > follows:
> > 	cdef setThis(self,  _Partition& other):
> > 		swap[_Partition](self._this,  other)
> > 		return self
> > 
> > where swap is defined as
> > 
> > cdef extern from "<algorithm>" namespace "std":
> > 	void swap[T](T &a,  T &b)
> > 
> > According to my experiments [0], the partition is not copied anymore.
> > 
> > I've just pushed the changes to the Dev branch.
> > 
> > I guess the same could be done for Graph and Cover which means that we
> > wouldn't need to use these "dereference" calls and we wouldn't need the
> > extra code for Cython anymore.
> 
>  So special code like this
> 
> 	/** only to be used by cython - this eliminates an unnecessary copy */
> 	Graph* _read(const std::string& path) {
> 		return new Graph{std::move(read(path))};
> 	};
> 
> would no longer be necessary? I am very much in favor of eliminating this
> kind of code, because it is complicated to explain. It’s also nicer to have
> all the Cython-related stuff in the .pyx file.
> 
> Michael, will you apply the appropriate changes to the Dev branch?

I've just applied the changes. From my tests it seems to work. I've tested the 
code with a Graph copy assignment operator and copy constructor implementation 
that throws an exception and I was able to generate and read Graphs. I've 
removed the special code for cython in the C++ classes where I've found it.

If you have custom classes in Cython you need to remove the dereference-
operator from all graph parameters and if you return graphs, you need to 
change the code to return a value instead of a pointer.

Please test and review the changes and report any problems.

Michael



More information about the NetworKit mailing list