[Octave-bug-tracker] [bug #54100] fread using SKIP larger than zero is extremely slow

[Dan Sebald]

Follow-up Comment #19, bug #54100 (project octave):

How about we address this while looking at this stream::read() function?


    // FIXME: Ensure that this does not overflow.
    //        Maybe try comparing nr * nc computed in double with
    //        std::numeric_limits<octave_idx_type>::max ();
    octave_idx_type elts_to_read = nr * nc;


Now, there's no need to check that nr or nc is too big, because that is done
already in 


  static octave_idx_type
  get_size (double d, const std::string& who)
[snip]
        if (d > std::numeric_limits<octave_idx_type>::max ())
          ::error ("%s: dimension too large for Octave's index type",
                   who.c_str ());


However, I get the point that the FIXME is raising.  An octave_idx_type is
being used to hold the size "elts_to_read = nr * nc" and that number could
potentially be larger than std::numeric_limits<octave_idx_type>::max.  How
about addressing this as follows:


    // Check for overflow.
    if (nr > 0 && nc > (std::numeric_limits<octave_idx_type>::max () / nr))
      error ("fread: dimension too large for Octave's index type");


(See
https://stackoverflow.com/questions/199333/how-to-detect-integer-overflow.)

One related item.  std::numeric_limits<octave_idx_type>::max is (as printed
out using std::cerr):

OIT MAX: 9223372036854775807

That's 9.2e18 or 2^63.  So it is already near 2^64, the addressable memory
limit...and that is 1 million tera-"elements".  First, given the practically
unachievable memory size, does it make sense for this error message:


octave:26> fid = fopen("zeros1000by61.dat","r"); tic; xt = fread (fid, [1
10000000000], 'single'); toc; fclose(fid);
error: out of memory or dimension too large for Octave's index type


to include the phrase "or dimension too large for Octave's index type"?  In
reality, from pt-eval.cc the std::bad_alloc really is only indicating
"insufficient memory", most likely.  (Could be some obscure thing with
user-account security, don't know.)  Here is the code:


        catch (const std::bad_alloc&)
          {
            // FIXME: We want to use error_with_id here so that give users
            // control over this error message but error_with_id will
            // require some memory allocations.  Is there anything we can
            // do to make those more likely to succeed?

            error_with_id ("Octave:bad-alloc",
                           "out of memory or dimension too large for Octave's
index type");


I'm careful to say "insufficient memory", as opposed to "out of memory" or "no
memory left" in regard to the FIXME comment just above.  This FIXME is making
it sound as though there is no memory left, but there probably is plenty of
memory left for error messages, just not enough for some huge vector.  So, I'm
not sure what the concern is.

Anyway, getting back on point, the "or dimension too large" seems a red
herring.  Can it be dropped?  Or maybe it is meant to be a suggestion, like
"out of memory (possibly dimension too large)"?  I don't know, is there
currently a custom "new" installed that can more specifically state that D1 x
D2 x ... x DN grows outside the index-type max.  But couldn't an error be
thrown prior to attempt the new?

Something else is odd here:


octave:9> fid = fopen("zeros1000by61.dat","r"); tic; xt = fread (fid,
[9223372036854775807, 9223372036854775807], 'char'); toc; fclose(fid);
OIT MAX: 9223372036854775807
nr: -9223372036854775808  nc: -9223372036854775808
(std::numeric_limits<octave_idx_type>::max () / nr): 0
nr * nc: 0
Elapsed time is 0.000128984 seconds.


The std::cerr << nc may be misinterpreting the sign of octave_idx_type, but
the overflow test I put in place (see above) doesn't fail...that's odd, and
nr*nc comes out to be zero.  Either it should rail to max octave_idx_type or
wrap around.

Trying to get away from possible sign/unsigned issues for nr and nc, let's
divide by 3:


octave:10> fid = fopen("zeros1000by61.dat","r"); tic; xt = fread (fid,
[9223372036854775807/3, 9223372036854775807/3], 'char'); toc; fclose(fid);
OIT MAX: 9223372036854775807
nr: 3074457345618258432  nc: 3074457345618258432
(std::numeric_limits<octave_idx_type>::max () / nr): 3
nr * nc: -8198552921648660480
input_buf_size: 1048576
error: out of memory or dimension too large for Octave's index type


OH, I confirmed that the overflow check I put in place is correctly flagging
the error, but somehow, somewhere that longer error message is appear instead
of the "fread: dimension too large for Octave's index type".  Is it because of
the try/catch of the interpreter?

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?54100>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/