octave-maintainers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Stories from static analysis of the Octave code base


From: Daniel J Sebald
Subject: Re: Stories from static analysis of the Octave code base
Date: Sat, 5 Jan 2019 11:32:26 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1

On 01/04/2019 02:35 PM, Rik wrote:
All,

It's been a humbling experience going through the issues detected by static
analysis of the Octave code base
(https://wiki.octave.org/PVS_static_analyzer_-_5.0_Release).  Frankly, I
thought the code base was better than it turned out to be.  Besides flat
out calling the wrong function (e.g., double_value instead of
xdouble_value), I just found and corrected a surprisingly simple but
pervasive performance hit for any exceptional value (-Inf, Inf, NaN).

The code in question is deep in liboctave at liboctave/util/lo-ieee.cc.
The representation of an IEEE exceptional value on the particular HW Octave
is running on is found and then stored away so that subsequent uses just
retrieve that value rather than re-deriving it each time.  The
initialization code was

void
octave_ieee_init (void)
{
   bool initialized = false;

   if (! initialized)
     {
        ... code to find and store exceptional values ...
        initialized = true;
     }
}

As you can see, initialized is always false so the longer code to derive
the exceptional values is run every single time.  And the exceptional
values themselves are functions which end up calling octave_ieee_init
twice: once for the "double" value and once for the "single" value.

The compiler should have issued a "test always false" type of warning for that...

There might be a small non-bug change in order beyond what you modified in Rev #26425. If I understand correctly, before the change something like

x = Inf(1000);

would have a performance hit because of the reinitialization for every instance of IEEE inf.

However, there are multiple locations where there is assurance of initialization:

libinterp/corefcn/interpreter.cc:    octave_ieee_init ();
liboctave/util/lo-ieee.cc:  octave_ieee_init ();
liboctave/util/lo-ieee.cc:  octave_ieee_init ();
liboctave/util/lo-ieee.cc:  octave_ieee_init ();
liboctave/util/lo-ieee.cc:  octave_ieee_init ();
liboctave/util/lo-ieee.cc:  octave_ieee_init ();
liboctave/util/lo-ieee.cc:  octave_ieee_init ();

The first check is when an instance of the interpreter is created, in the constructor. The remaining are when there is a particular call to actually set +/-Inf or NA. I would think that only one of those two groups is needed, but not both.

If the IEEE initialization is checked only once upon creating the interpreter then Octave would avoid quite a bit of checking in the x=inf(1000) case, but that would also mean that Octave would immediately abort if the Inf/NA aren't present when an interpreter is created for any particular non-Inf/NA script.

Instead, if the IEEE initialization is checked only whenever an Inf/NA assignment is done, then Octave would have a chance to run properly so long as it didn't need Inf/NA.


Also, despite their name, Octave uses exceptional values quite a bit.  For
example, graphics handles are often initialized to NaN until they can be
assigned a true handle.

Then that makes more of a case for having the octave_ieee_init() as mandatory in interpreter.cc. [One could temporarily force the occurrences of octave_ieee_init() in lo-ieee.cc to abort and see if that function is used anywhere outside the interpreter prior to instantiation of the interpreter.]

Something else I'm wondering about is what would happen in the case where there is no octave_ieee_init() in interpreter::interpreter() and the NA/Inf arises via CPU/ALU arithmetic operations (e.g., 1 DIV 0) as opposed to assignment. In other words, if there were no instance of octave_ieee_init() in interpreter::interpreter, could there be situations where the non-IEEE float fails even when none of the abort-plus-informative-message checks happen and one simply gets abort-via-crash?

And what about within lo-ieee.cc itself? There are currently no octave_ieee_init() within the routines

__lo_ieee_is_NA (double x)
__lo_ieee_is_old_NA (double x)
__lo_ieee_float_is_NA (float x)

Couldn't the user run a script, say "isinf(5)" or "isnan(20)", before actually creating a variable with value +/-Inf or NA?

Dan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]