Re: move constructors likely a requirement

[Lars Winterfeld]

Am 24.08.19 um 07:01 schrieb Rik:
> --- Code bm.assign.m ---
> runs = 5;
> 
> cumulate = 0; b = 0; z = 0;
> for i = 1:runs
>   b = zeros (620, 620);
>   tic;
>     for j = 1:620
>       for k = 1:620
>         z = 13;
>       end
>     end
>   timing = toc;
>   cumulate = cumulate + timing;
> end
> 
> ## Total time
> cumulate
> --- End Code ---
> 
> The only thing it does is assign the static value 13 to the variable z.  In
> 3.2.4 this took 0.17 seconds and now it takes 0.96.
> 
> I think I would start investigations with this script.  Is it the creation
> of a new octave_value for 13 every time?  Is it assignment?


Hi,

maybe you do this already, but what you can do to benchmark is:
(under Linux, have a few minutes patience)

valgrind --tool=callgrind octave-cli -q ~/bm.assign.m
kcachegrind "$(ls -1t callgrind.out.* | head -1)"

I have octave 4.2.2 installed (Ubuntu version, without debug symbols).
On first sight, those functions take time in the bm.assign.m example:

octave_lvalue::assign(octave_value::assign_op, octave_value const&)

octave::tree_evaluator::visit_simple_for_command(tree_simple_for_command&)

octave::tree_evaluator::visit_statement(tree_statement&)

tree_simple_assignment::rvalue1(int)

(Makes sense, the code is mainly makes assignments.)

I understand that legacy code sometimes abuses a "pass by reference" to
return a value, but I would advise against formulating it as a new
recommendation. In modern C++, you can easily return objects (or even
tuple of objects). Maybe check if you are compiling with C++14 or higher
(I believe the requirements of return value optimization were relaxed at
some point).

Haven't looked at the code, but I'd also advise to use more move
semantics. Just to spelled it out, in the above example this would
involve writing the move constructor
visit_simple_for_command(visit_simple_for_command&&)
(and maybe a move assignment) and say "a = std::move(b);" on the last
use of b. Or push_back(move(b)), etc.

Likewise
octave_lvalue::assign(octave_value::assign_op, const octave_value&&)
and use
assign(op, move(val));
when you no longer need val afterwards.

Hope it helps,
Lars