Re: move constructors

[Rik]

On 08/15/2019 11:51 AM, John W. Eaton wrote:
> On 8/14/19 6:31 PM, Rik wrote:
>> On 08/14/2019 02:43 PM, Rik wrote:
>
>>> Just to get an idea, I modified the destructor in Array.h to
>>>
>>>     ~ArrayRep (void) {
>>>         static long int n = 0;
>>>         std::cerr << "~ArrayRep: " << ++n << "\n";
>>>         delete [] data; }
>>>
>>> I find that running an average statement like "x = 1;" causes 40
>>> destructor calls.  That does seem excessive.
>
> Yeah, that does seem like a lot.  Maybe it can be reduced, but first we
> have to know where they are happening and why.

Yes, this was mostly about confirming hunches.

>
>> A similar experiment with modifying the destructor for octave_value_list
>> shows that we are creating/destroying a lot more of these then I think
>> is necessary given the complexity of the statements.  A simple function
>> call seems to require 9 octave_value_list objects.
>
> I'm sure there's a lot of room for improvement.  I traced the calls to
> the destructors for the expression "sin(1);".  I found:
>
>   * Three calls in tree_evaluator::convert_to_const_vector.  That is the
> function that takes the argument list object in the parse tree and
> converts it to an octave_value_list object.
>
>   * Five calls in tree_evaluator::visit_index_expression.  Two of those
> are happening at the point of the call to the sin function, so probably
> inside the code that calls the function, not the visit_index_expression
> function itself.
>
>   * One more call occurs inside the tree_evaluator::evaluate function
> when re-initializing the m_expr_result_value_list value.
>

The good news, as you say, is that there is plenty of room for
optimization.  Switching the octave_value_list class to base itself atop
std::vector might get 25% performance improvement.  But optimizing the
design, by calling the octave_value_list constructor only once versus nine
times, could get an 800% improvement.  We won't really get that low, but
far more opportunities if we optimize the design rather than the code.

It seems the first two functions are in pt-eval.cc and the final one in
pt-eval.h.  I was confused because there is another
tree_evaluator::evaluate function in pt-eval.cc, but it makes no reference
to m_expr_result_value_list.  For reference, how did you determine which
functions were causing the calls?  A debugger and a breakpoint in
~octave_value_list()?

>> y =  2
>> ~octave_value_list: 7814
>> ~octave_value_list: 7815
>> ~octave_value_list: 7816
>> ~octave_value_list: 7817
>> ~octave_value_list: 7818
>> ~octave_value_list: 7819
>
> Not that it's the only issue, but as far as I can tell, these are all due
> to calling the disp and display functions to print the result.  If I
> execute "y = 2;" to suppress printing, there are no calls to the
> octave_value_list destructor.
>

Ah, that is comforting.  Running data processing scripts in batch mode will
not need to print out results and so can run fast.  And if a user is
running Octave interactively and inspecting results then an extra
millisecond delay is not noticeable.

--Rik