Implement move semantics for Add and Mul

[certik]

This PR implements move semantics in Add's and Mul's constructors, so the dictionary (std::map for Mul or std::unordered_map for Add) is not copied when the class is constructed, but rather it is moved using the C++11 move semantics using rvalue references.

Just google "rvalue references", e.g. here is some introduction: http://thbecker.net/articles/rvalue_references/section_01.html. It is confusing at first, but the actual practice, as seen from this PR, is pretty simple. One just has to be careful to make sure the object really gets moved, instead of copied by proper using of std::move everywhere, as well as proper declarations.

This significantly speeds up some of our benchmarks (i.e. 12% speedup):
Before:

ondrej@kittiwake:~/repos/csympy/benchmarks((cc8cb8b...))$ ./expand2
Expanding: ((y + x + z + w)^15 + w)*(y + x + z + w)^15
1211ms
number of terms: 6272
ondrej@kittiwake:~/repos/csympy/benchmarks((cc8cb8b...))$ ./expand2
Expanding: ((y + x + z + w)^15 + w)*(y + x + z + w)^15
1229ms
number of terms: 6272
ondrej@kittiwake:~/repos/csympy/benchmarks((cc8cb8b...))$ ./expand2
Expanding: ((y + x + z + w)^15 + w)*(y + x + z + w)^15
1194ms
number of terms: 6272
ondrej@kittiwake:~/repos/csympy/benchmarks((cc8cb8b...))$ ./expand2
Expanding: ((y + x + z + w)^15 + w)*(y + x + z + w)^15
1184ms
number of terms: 6272
ondrej@kittiwake:~/repos/csympy/benchmarks((cc8cb8b...))$ ./expand2
Expanding: ((y + x + z + w)^15 + w)*(y + x + z + w)^15
1186ms
number of terms: 6272

After:

ondrej@kittiwake:~/repos/csympy/benchmarks(move)$ ./expand2
Expanding: ((y + x + z + w)^15 + w)*(y + x + z + w)^15
1080ms
number of terms: 6272
ondrej@kittiwake:~/repos/csympy/benchmarks(move)$ ./expand2
Expanding: ((y + x + z + w)^15 + w)*(y + x + z + w)^15
1068ms
number of terms: 6272
ondrej@kittiwake:~/repos/csympy/benchmarks(move)$ ./expand2
Expanding: ((y + x + z + w)^15 + w)*(y + x + z + w)^15
1062ms
number of terms: 6272
ondrej@kittiwake:~/repos/csympy/benchmarks(move)$ ./expand2
Expanding: ((y + x + z + w)^15 + w)*(y + x + z + w)^15
1044ms
number of terms: 6272
ondrej@kittiwake:~/repos/csympy/benchmarks(move)$ ./expand2
Expanding: ((y + x + z + w)^15 + w)*(y + x + z + w)^15
1058ms
number of terms: 6272

@hazelnusse, let me know what you think. Do you have more ideas along these lines how to speedup CSymPy?

@thilinarmtb, @sushant-hiray if you could review this PR as well, that would be awesome.

[certik]

(Travis tests are passing: https://travis-ci.org/certik/csympy/builds/27439688.)

[certik]

For comparison, on the same machine.

Ginac:

(y+z+w+x)^15
(y+z+w+x)^15*((y+z+w+x)^15+w)
1531ms

Sage:

$ /scratch/sage-6.2-x86_64-Linux/sage expand2_sage.py 
((w + x + y + z)^15 + w)*(w + x + y + z)^15
Total time: 7.06153416634 s

[thilinarmtb]

I went through the Thomas Becker`s article couple of times, I don't think I understood it completely. But, to my knowledge, this PR is +1 to merge.

[certik]

I don't understand it completely either. But the relatively simple changes
in this PR seem to be confirmed by it.

Sent from my mobile phone.
On Jun 13, 2014 8:57 AM, "Thilina Bandara Rathnayake" <
notifications@github.com> wrote:

I went through the Thomas Becker`s article couple of times, I don't think
I understood it completely. But, to my knowledge, this PR is +1 to merge.

—
Reply to this email directly or view it on GitHub
#192 (comment).

[thilinarmtb]

I tried to benchmark expand2 without using the temporaries d2 and d3 here and in pow.cpp. I got better results once or twice but most of the times the timings were not that different. Removing d2 and d3 results in quite unreadable code anyway, so I don't think it matters a lot.

[certik]

You have to always do lots of runs and take the fastest time. The numbers always oscillate a bit. The temporaries d2 and d3 should be optimized out by the compiler. It's just a reference anyway (no copying being done).

[thilinarmtb]

Yes, I didn't do a lot of test runs on this. Compiler should optimize them so I guess there is not going to be a huge difference.

[thilinarmtb]

Yes, I agree.

[sushant-hiray]

I just read about std::move and it is pretty interesting and seems quite intuitive.
We got quite a significant improvement. I guess the PR looks quite good.
We should perhaps look more into such features added by C++11 to see if we can use them in our case.

[certik]

Ok, there seem to be no objections, tests pass, so I am merging this.