Re: assert () taking long time

[Rik]

On 09/20/2013 03:22 AM, Daniel Kraft wrote:
> Hi,
>
> On 2013-09-19 22:06, Rik wrote:
>> Are you working from the development version of Octave in the Mercurial
>> repository?  The assert function has been massively re-written on the
>> development branch and it may, or may not, still have performance
>> issues.  Assuming you are using the latest assert.m, what kind of
>> performance hit are you seeing?  Is it 1% or 10% of your script running
>> time?
> I didn't, but I now updated to the current hg head (as of this morning)
> and the behaviour is the same.  If I enable all assert() calls (which I
> already manually disabled in the places where most of the time was
> spent) and do some typical calculation with my code, it takes nearly 50%
> of the time (assert and deblank combined):
>
> octave:2> profshow(profile('info'))
>    #         Function Attr     Time (s)        Calls
> ----------------------------------------------------
>  123           assert            46.269       510057
>  125          deblank            25.949       510066
>  211 findMeshVertices            18.137            3
>  171    buildMatrices             5.205            5
>  310         __line__             4.338         1690
>  312      __go_line__             3.865         1690
>   15           nargin             2.531      2138678
>   75             find             1.990       539172
>   12              max             1.960       522487
>   16        binary ==             1.857      2466308
>
> tic/toc reports 155 seconds for the whole execution.  The asserts in
> question are within loops but only on simple conditions (which may
> involve things like length() or indexing a vector, but not comparisons
> of huge matrices or so).
>
>> For the version in the repository, the calls to deblank could easily be
>> avoided.  We pre-calculate an error message using deblank, but only
>> print it if there is a problem.  You could move the calculation of the
>> message inside the error checking code so that it is only generated if
>> needed.  Look for the variable "in" in the code; It is used in two if
>> branches so you would need to duplicate it for each if body, but that is
>> simple.
> I think it should be even acceptable to replace the custom error message
> by something like "assertion failed" without details if the user
> explicitely specifies this via a configuration option for production
> runs, no?
>
> Here are two more tests:  When I add a custom assert.m which is just an
> empty function, I get a total time of 56 seconds and
>
>    #                              Function Attr     Time (s)
> ------------------------------------------------------------
>  205                      findMeshVertices            14.553
>  166                         buildMatrices             5.239
>  306                              __line__             4.335
>  308                           __go_line__             3.844
>  223                               cellfun             1.791
>  125                              bdrygeom             1.448
>  182                                ls2sd2             1.229
>
> Even if assert is implemented like this:
>
> function assert(cond)
>   if (~cond)
>     error ("assertion failed");
>   endif
> endfunction
>
> which has all the "base" functionality (at least the one I need/use),
> then the time is still only 57 seconds with
>
>    #                              Function Attr     Time (s)
> ------------------------------------------------------------
>  205                      findMeshVertices            14.509
>  166                         buildMatrices             5.222
>  306                              __line__             4.304
>  308                           __go_line__             3.714
>  223                               cellfun             1.782
>  125                              bdrygeom             1.466
>  182                                ls2sd2             1.257
>  184                        ls2sd2>dKKTsys             1.086
>
> So what about simply adding an option that reduces assert() to basically
> this implementation in case we just have a single argument?  For my
> situation, this would already improve it to the point that I wouldn't
> any longer need to disable asserts manually.  Of course it is trivial to
> replace assert as I just did, but it would be great if this was possible
> also with Octave's built-in version.
>
> Yours,
> Daniel
>
9/23/13

Daniel,

I just checked in a changeset which avoids pre-calculating the error
message which may never get used (~16% speed-up).  I also did a little
profiling and changed a repmat construction to a const*ones(...) (~23%
speed-up).  So, overall maybe a 33% improvement.  I think if you want to
substitute a much simpler, albeit faster, version of assert you should make
it a local change.  The assert.m in Octave needs to stay general and able
to work for simple logical expressions, numeric comparisons, on up to
recursive comparison of cell arrays and structures.  Perhaps the easiest
thing would be to rename your function to qassert.m (Quick Assert) and then
find and replace all instances of assert in your code with qassert.

Cheers,
Rik