bugGNU Octave - Bugs: bug #51871, loading '-ascii' format files is...

 
 

bug #51871: loading '-ascii' format files is slow

Submitter:  Eddy <count>
Submitted:  Sun 27 Aug 2017 06:15:42 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Performance
Status:  In Progress Assigned to:  None
Originator Name:  Open/Closed:  * Open
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 27 Nov 2017 06:03:00 AM UTC, comment #46: 

No hurry...  The easiest way to highlight what I've modified (which really isn't that much) is to run

meld speed-up-load-ascii-v5.patch speed-up-load-ascii-v8.patch &
(make a getline version that handles all EOL, i.e., 'EOL_tst.txt' example)

and

meld speed-up-load-ascii-v4.patch speed-up-load-ascii-v5.patch &
(handle the overflow/underflow ranges with proper sign, i.e., 'ERANGE_tst.txt' example)

Keep in mind that my generated patch files may have had the minimum diff hunk size different than someone else's settings, so in meld there are often a bunch of adds/removes which are effectively no change.

Dan Sebald <sebald>
Mon 27 Nov 2017 05:40:49 AM UTC, comment #45: 

@Dan: thanks for all this work.  I will have to check through it carefully, which probably means next weekend before I can grab a big enough block of time.

Rik <rik5>
Group administrator
Sun 26 Nov 2017 06:55:27 PM UTC, comment #44: 

"With that, I think there isn't too much more inefficiency that can be squeezed out of this one"

Ah, not exactly.  The


  std::istream::sentry se(is, true);


can be moved outside of the getline_alleol_sanscomment() function and just called once prior to the do loops scanning every line.  It saves a few percentage points, time-wise, but that is no longer a drop-in replacement for getline(), so I didn't include that in the speed-up-load-ascii-v8.patch patch.

Dan Sebald <sebald>
Sun 26 Nov 2017 06:39:12 PM UTC, comment #43: 

OK, another rev attached (speed-up-load-ascii-v8.patch).  I did pursue the alteration of a stream buffer filter, which I think is very elegant, but I abandoned that approach and stuck with the custom-getline stand-alone function approach.  There is an advantage to the custom-getline, which I will illustrate, plus I think it is easier to follow for any programmer who might otherwise not follow that there is some type of filter in stream.

The key piece of information to speeding the custom-getline (and filter-stream-buffer underflow routine...pretty much the same code concept), is the understanding that working with the istream's stream buffer directly is faster than working with the istream's next higher level of methods.  This is explained in the comment of the example given here:

https://stackoverflow.com/a/6089413
https://stackoverflow.com/questions/6089231/getting-std-ifstream-to-handle-lf-cr-and-crlf

In the attached patch I have pre-processor "#if 0" around three different variations of the custom-getline, the first in which I applied a trick from ASCII table properties, i.e., most characters of interest are greater than '%' > '#' > '\r' > '\n' > EOF and two other variations that are patterned on a switch statement similar to the example in the link above.  Take your pick, and maybe the variations will give someone else an idea for efficiency improvements.  [Why count's custom getline based on a stream buffer directly turned out slower, I don't know.  Post the code if you like.]

So, the advantage of custom-getline?  As you might guess, seeing as I listed some comment characters above, it is that we can move the handling of comments within the custom-getline and eliminate the inefficient secondary search (through the whole string), i.e.,


      // Remove any comment.
      size_t pos_comment = retval.find_first_of ("#%");
      if (pos_comment != std::string::npos)
        retval.erase (pos_comment);


Hence, I've called the custom-getline


  getline_alleol_sanscomment (std::istream& is, std::string& str)


Rik, you'll have to let me know if # or % can be valid characters within some string field of the ASCII data.  If so, then the custom comment removal will need to be made more complex, but I don't think it would hurt performance any.

So, with that.  The new version now outperforms the variation that uses the standard library getline():

current octave: 3.7360, 3.7440
octave + speed-up-load-ascii-v5.patch: 1.3640
octave + speed-up-load-ascii-v6.patch: 2.1980
octave + speed-up-load-ascii-v7.patch: 3.1040
octave + speed-up-load-ascii-v8.patch: 1.2240

With that, I think there isn't too much more inefficiency that can be squeezed out of this one, without going to some lower level methods.  This ASCII load is just one of about four methods currently using an istream object as an input, so I would tread lightly with trying to use C-level I/O and alter the input object of all those other methods.

Oh, I should add that both of the test files pass for version speed-up-load-ascii-v8.patch:


octave:13> x = load('EOL_tst.txt')
x =

   1   2   3
   4   5   6
   7   8   9
   1   2   3
   4   5   6
   7   8   9

octave:14> x = load('ERANGE_tst.txt')
x =

   1.0000e+128  -1.0000e+128
   1.0000e+256  -1.0000e+256
           Inf          -Inf
           Inf          -Inf


(file #42492)

Dan Sebald <sebald>
Sat 25 Nov 2017 08:04:31 PM UTC, comment #42: 

Attached are a few more incremental versions,


speed-up-load-ascii-v6.patch  (uses istream is.get(), clear string)
speed-up-load-ascii-v6v2.patch  (uses istream is.get(), overwrite string)
speed-up-load-ascii-v7.patch  (uses istream is.read(,1))


The v6v2 version yields the same time as the v6 version, so that confirms what you said about the string clear() function done in a fairly efficient way.  (I actually had done this comparison before your last post.)

Speed comparisons:

current octave:  3.7360, 3.7440
octave + speed-up-load-ascii-v5.patch:  1.3640
octave + speed-up-load-ascii-v6.patch:  2.1980
octave + speed-up-load-ascii-v7.patch:  3.1040

The above also agrees with your comment about the standard getline() being optimal.  Note above how version v5 is clearly much faster.  No surprise, as calling a routine get() for individual characters is going to have overhead.  But version v5 doesn't handle all EOL characters.

In version v7 I used read(,1) instead of get().  That slows down, but it is obvious why.  Although read(,1000) is much faster than call get() 1000 times, read(,1) has the extra overhead of a second input variable on the stack; it has to be slower than get().

So far, then, version v6 is the benchmark of fastest while still handling all EOL.

I thought to pursue reading in data at bigger hunks, and then search for EOL characters.  It seems too clumsy though, so I hesitate.  I then thought to perhaps go back to FILE * and lower level C-like I/O, but that messes far too much with other text/matrix/etc. code which is based on istream objects.

The following reference suggests an option that seems much more straightforward and efficient:

https://stackoverflow.com/questions/13995971/using-get-line-with-multiple-types-of-end-of-line-characters

The idea would be to create a "filter buffer stream" for which the istream is buffer passes through.  That filter buffer stream will convert all 0x0A, 0x0D, 0x0D-0x0A characters to the native '\n' character, then we can use getline() just as it is.  That is

istream is --> filter istream fs --> fs.getline()

That seems the most efficient and elegant solution, doesn't it?  At least in principle.  I'm going to try coding that.  If it doesn't work, then version v6 it is, I guess.

(file #42485, file #42486, file #42487)

Dan Sebald <sebald>
Sat 25 Nov 2017 08:14:27 AM UTC, comment #41: 

Reply for comment #35:
  It is enough to just call clean() for std::string to clean the data, just like std::vector, it is optimized. BTW: assign an empty string is a slower way to clean it.
  Also this is what std::getline do. See https://gcc.gnu.org/onlinedocs/libstdc++/latest-doxygen/a00293_source.html
    __str.erase();


Reply for comment #34:
  push_back() is fast, all slow thing is around std::istream.


I also tried implement a customized "getline" that accept all line endings, by adopting the std::getline code. It operates in a lower level that deal with (std::)stream buffer directly.

The surprising thing is: it is much slower (I can't recall the timing, maybe later) than the std::getline even after I remove all the customizations, that's say the GCC compiler has a specific optimization for std::getline, and this optimization can not be triggered by simply copy the std::getline implementation.

Eddy <count>
Sat 25 Nov 2017 06:28:31 AM UTC, comment #40: 

Here's a reference that compares various file input methods:

https://stackoverflow.com/questions/18688763/why-is-istream-ostream-slow

Anything that brings in one individual character at a time is slow.  However, note that the person's example is kind of simplified and suited for reading large hunks of data at once.  That is, it's not geared to reading a line of ascii text, then another, then another, then another.

I've implemented some of the methods I outlined in previous posts.  They aren't as slow as the current code, but they are about 50% slower than speed-up-load-ascii-v5.patch (which uses getline).

I have an idea though.  What if I write getline_alleol() to bring in a hunk of data?  One asks, How do we know the size of the hunk to bring in?  If we read too much, then seek back to the EOL (0x0A, 0x0D, 0x0D/0x0A), that becomes inefficient.  Too short and that becomes inefficient as well.  How about choosing the hunk size to be the length of the last line read?  That should be a good initial guess.  So long as the data is fairly consistent in its field width from line to line, this should be efficient.

Dan Sebald <sebald>
Sat 25 Nov 2017 04:25:27 AM UTC, comment #39: 

Attached is a data file with huge out-of-range numbers.  The patch appears to work properly:


octave:3> x = load('ERANGE_tst.txt')
x =

   1.0000e+128  -1.0000e+128
   1.0000e+256  -1.0000e+256
           Inf          -Inf
           Inf          -Inf


(file #42476)

Dan Sebald <sebald>
Sat 25 Nov 2017 03:57:50 AM UTC, comment #38: 

Efficiency always counts.  In this age of big data where someone might want to use Octave to analyze thousands of files, there's a big difference between, say, a 2 hour batch process and 8 hour batch process.

The speed-up-load-ascii-v4.patch does not compile, so I'm attaching a speed-up-load-ascii-v5.patch which is the same as speed-up-load-ascii-v4.patch with the following additional hunk:


   std::ios::iostate status = is.rdstate ();
   if (status & std::ios::failbit)
     {
-      // Convert MAX_VAL returned by C++ streams for very large numbers to Inf
-      if (val == std::numeric_limits<T>::max ())
+      // Convert +/- HUGE_VALF returned by C++ strtof for very large numbers to Inf
+      if (val == HUGE_VALF)
         {
-          if (neg)
-            val = -std::numeric_limits<T>::infinity ();
-          else
-            val = std::numeric_limits<T>::infinity ();
+          val = std::numeric_limits<T>::infinity ();
+          is.clear (status & ~std::ios::failbit);
+        }
+      else if (val == -HUGE_VALF)
+        {
+          val = -std::numeric_limits<T>::infinity ();
           is.clear (status & ~std::ios::failbit);
         }
       else


For reference on HUGE_VALF, see

http://www.cplusplus.com/reference/cmath/HUGE_VALF/
http://www.cplusplus.com/reference/cstdlib/strtof/?kw=strtof

(ERANGE could have been used to determine either HUGE_VALF or -HUGE_VALF, but I didn't want to complicate the use of failbit at the moment.)  I've not tested this with any out-of-range data, but that's not the focus right now.

(file #42475)

Dan Sebald <sebald>
Thu 23 Nov 2017 03:20:40 PM UTC, comment #37: 

@Dan: You had some ideas regarding this report.  I think if testing those ideas were quick it could be worth experimenting.  The counter-argument is that the code already works, and performance is a secondary goal after correctness.  Even though it is slightly annoying to have to wait 3 seconds rather than 0.5 seconds as with the C++ standard library, 2.5 seconds is not really going to ruin anyone's day.  And there are lots of bug reports, for example those with gnuplot, which you are specially qualified for solving.  So if this issue remains open and you want to work on other matters of correctness I understand.

Rik <rik5>
Group administrator
Thu 09 Nov 2017 01:44:52 AM UTC, comment #36: 

Just wondering if here:


+      // Error during conversion.
+      if (str_chunk.size () >= 2
+          && std::toupper (str_chunk[0]) == 'N'
+          && std::toupper (str_chunk[1]) == 'A')
+        {
+          // Found NA
+          val = octave::numeric_limits<T>::NA ();
+          end_ptr += 2;
+        }


the test should have "str_chunk.size() == 2" because otherwise this test would pass for things like NATION, NASCENT, etc.

Dan Sebald <sebald>
Wed 08 Nov 2017 10:31:43 PM UTC, comment #35: 

I guess somewhere in that getline_alleol() would need to be a clearing of the string str, but I'm wondering if we could avoid the clearing of the string str and instead reuse it's current contents until it is necessary to expand it.  Say something like below.  The idea is to overwrite the existing buffer until it is necessary to expand it.  Then, at the end of the current line I've put a 0x0D EOL character in place so that subsequent routines which extract floating point data will end at the EOL.  That's the general idea.  I wonder if that would be more efficient than clearing the string buffer and continually using push_pack to expand it.


istream& getline_alleol (istream& is, string& str) {
    int c;
    int i;
    int strsize = str.size();

    for (i=0; i < strsize; i++) {
        c = is.get ();
        if (c == EOF || c == 0x0A)
            break;
        elseif (c == 0x0D) {
            if ((c = is.peek ()) != EOF) {
                 if (c == 0x0A)
                     is.ignore ();
            }
            break;
        } else
            str.replace (i,  1,  &c);
    }

    if (i == strsize) {
        while ((c = is.get ()) != EOF) {
            if (c == 0x0A)
                break;
            elseif (c == 0x0D) {
                if ((c = is.peek ()) != EOF) {
                    if (c == 0x0A)
                         is.ignore ();
                }
                break;
            } else
                str.push_back(c);
        }
       str.push_back (i,  1, 1, 0x0D);
    } else
       str.replace (i,  1, 1, 0x0D);

    return is;
}


Dan Sebald <sebald>
Wed 08 Nov 2017 09:34:54 PM UTC, comment #34: 

A lot of this is no longer fresh in my mind.  But if you can get the patch back on track I can look more closely.  I believe you are referring to speed-up-load-ascii-v4.patch.  Also, looking at your EOL_tst.txt in hexedit, I see that

0A
0D
0D,0A

are the valid EOLs.  So, my thinking, looking at the current state of the patch, is that rather than

+      std::getline (is, retval);

all that need be done is devise a custom version of that function that handles all EOLs, call it getline_alleol(is,retval).

According to the documentation

http://www.cplusplus.com/reference/string/string/getline/

"
Each extracted character is appended to the string as if its member push_back was called.
"

Is this push_back() routine efficient?  That is, does the string object expand its buffer on something other than a linear way, say doubling the size it needs?  If so, it seems to me that one could write a string-based routine utilizing push_back() in a fairly straightforward fashion:

istream& getline_alleol (istream& is, string& str);

i.e., extract a character at a time from istream is and test against the three EOL scenarios above and if none of those then use str.push_back(c).  Something like (there are other variants of the istream.get() routine):


istream& getline_alleol (istream& is, string& str) {
    int c;
    while ((c = is.get ()) != EOF) {
        if (c == 0x0A)
            break;
        elseif (c == 0x0D) {
            // Maybe this next check is extraneous if the
            // consequence of an empty line, i.e.,
            // 0x0D<emptyline>0x0A is simply that it is
            // ignored at higher levels, in which case the
            // 0x0A will be processed in the next pass.
            // Otherwise, there is slight ambiguity of
            // accepting any of the three EOL formats.
            if ((c = is.peek ()) != EOF) {
                 if (c == 0x0A)
                     is.ignore ();
            }
            break;
        } else
            str.push_back(c);
    }

    return is;
}


Dan Sebald <sebald>
Wed 08 Nov 2017 07:33:14 PM UTC, comment #33: 

I finally got a test of whether Matlab processes EOL characters once per file, or on every single line.  See comment #25 on this bug report, and comment #3 on bug #52355.

The answer is that Matlab appears to do per-line processing.  That still doesn't require Octave to do character by character processing, but it does make things harder.

@Dan: What parts of the patches attached to this report can we salvage and still implement?

Rik <rik5>
Group administrator
Fri 08 Sep 2017 11:04:49 PM UTC, comment #32: 

OK.  Fine as is.

True about 'status' always being used if there is a fail, but most of the time there is no fail, so it just saves a few cycles of not putting a value on the stack unless it is needed.

>> There's no real reason is.get () would start returning a space (' ') on an EOF or error case. Thus, the while loop will terminate eventually.


I can't find anything about what happens if there is a fail, but not an EOF type of fail, i.e., whether c1 is overwritten with something even if there is 'bad' stream.  Because c1 is initialized to ' ', if in the first is.get() there is some bad read, could c1 stay at ' '?  What scenario is considered a bad stream, I don't know.

Dan Sebald <sebald>
Fri 08 Sep 2017 10:43:23 PM UTC, comment #31: 

There's no real reason is.get () would start returning a space (' ') on an EOF or error case.  Thus, the while loop will terminate eventually.

Also, I'm pretty sure you don't want to check eof.  If the large value is the very last value in the file then


  is >> val;


will set both the fail bit and the eof bit.  Then this test will say that a true error occurred, rather than just a large value being encountered.


+      if (!is.bad () && !is.eof () && val == std::numeric_limits<T>::max ())


I went with the original coding strategy, rather than use is.fail(), because as you'll notice in your updated patch is.readstate is called in both the if and else branches.  If it is common to both branches one might as well just pull it up and out.


-  std::ios::iostate status = is.rdstate ();
-  if (status & std::ios::failbit)
+  if (is.fail ())
     {
       // Convert MAX_VAL returned by C++ streams for very large numbers to Inf
-      if (val == std::numeric_limits<T>::max ())
+      if (!is.bad () && !is.eof () && val == std::numeric_limits<T>::max ())
         {
           if (neg)
             val = -std::numeric_limits<T>::infinity ();
           else
             val = std::numeric_limits<T>::infinity ();
-          is.clear (status & ~std::ios::failbit);
+          is.clear (is.rdstate () & ~std::ios::failbit);
         }
       else
         {
           // True error.  Reset stream to original position and pass status on.
+          std::ios::iostate status = is.rdstate ();
           is.clear ();
           is.seekg (pos);
           is.setstate (status);



Rik <rik5>
Group administrator
Fri 08 Sep 2017 10:06:27 PM UTC, comment #30: 

Actually, in this reference it sounds like 'fail' is set when there is an EOF:

http://www.cplusplus.com/reference/istream/istream/get/

"
The first signature returns the character read, or the end-of-file value (EOF) if no characters are available in the stream (note that in this case, the failbit flag is also set).
"

Looking at this a little more closely, I'm wondering if the tests for validity should be moved sooner in octave_read_fp_value(), right after reading rather than after processing the character.

What led me to the above link is that I was wondering what is returned by is.get() when there is a failure.  Take a look at this current construct:


  char c1 = ' ';

  while (isspace (c1))
    c1 = is.get ();


There is no checking of valid read in this while loop, so I was wondering what happens if there is an I/O failure.  Will c1 get updated to something other than ' '?  If not, this will be an infinite loop.  From the documentation, it sounds like c1 is the EOF character when EOF is found, but what about other scenarios of bad/fail bits on the initial try?

And if there are failures, it seems likely that in the switch command that is.putback(c1) could get called.  If there were errors, it might be compounding things to do a stream operation on "is" if it is in a fragile state, i.e., maybe it would create a type of error different from the original error.

Anyway, for certain the EOF should be checked, so I'm updating the patch.

(file #41768)

Dan Sebald <sebald>
Fri 08 Sep 2017 09:29:36 PM UTC, comment #29: 

Just looking at the changeset.  For reference, rdstate is described here:

http://www.cplusplus.com/reference/ios/ios/rdstate/

It looks like there is shorthand for


   if (status & std::ios::failbit)


could be replaced by


   if (is.fail ())


Also, in the table at the above link it looks like "bad" i/o operation also sets the fail bit.  From reading this description

http://www.cplusplus.com/reference/ios/ios/bad/

it seems like float value out of range wouldn't be in the "bad" category.  So I'm attaching a patch for you to consider that checks that is.bad() isn't true.  Given that, it would probably be OK to change


+          is.clear (is.rdstate () & ~std::ios::failbit);


to a simple


+          is.clear ();


because EOF is not set, I suspect, (and we know 'bad' bit is not set because we checked for that) but it's fine as is.

(file #41767)

Dan Sebald <sebald>
Fri 08 Sep 2017 08:29:19 PM UTC, comment #28: 

As an intermediate step, because we still have no test of the file EOL_tst.txt, I modified Octave to return +/-Inf rather than +/-MAX_VAL when reading very large numbers from a file.  See cset http://hg.savannah.gnu.org/hgweb/octave/rev/1cb94f46466f.

Rik <rik5>
Group administrator
Fri 01 Sep 2017 06:42:35 PM UTC, comment #27: 

Just some thoughts on this after experimenting with some code mods.  Using a very rough estimate of the example script taking 4 seconds,

1) Removing the get_lines_and_columns() and replacing with manual nr=1e6, nc=1 reduces the time to 3 seconds.

2) Having

          std::string buf = get_mat_data_input_line (is);

in the loop adds 0.5 seconds.  (I.e., I just comment out the above line and set buf = "1.234" before the loop.)

3) I used

          d = ::atof(buf.c_str());

to scan the float, and its contribution seems imperceptible.  But I don't think this scan

 d = octave_read_value<double> (tmp_stream);

takes too long either.  Hence, the actual scanning of the data to floats is not a bottleneck.

4) If I take all the looping out of the read_mat_ascii_data() by setting NR to 1, there is still 0.9 seconds consumed CPU.

5) The creation of the 1e6 x 1 Matrix doesn't seem to take much time.

6) That leaves about 1 to 1.5 seconds associated with this

          std::istringstream tmp_stream (buf);

Avoid using such a construct.

7) Why the 0.9 seconds then?  Well, if I reduce the contents of the dat.txt file from 1e6 lines to just 1 line, the time goes to 0.00718498 seconds.  So, it is actually this line in load-save.cc:

            {
              std::ifstream file (fname.c_str (), mode);

              if (! file)
                error ("load: unable to open input file '%s'",
                       orig_fname.c_str ());

that is contributing a big amount of time.  Does that make sense?  The process of opening a large file or creating a stream from a large file takes considerable time?  My experience doesn't suggest that just opening a file in C doesn't take much CPU.

Here is a link to general comments about streams:

https://stackoverflow.com/questions/26095160/why-are-stdfstreams-so-slow

8) Also, having sped up the load(), I notice that save() prior to the tic/toc takes a long time.  Does save() have the same issue, i.e., using streams is slow?  I would think save() is much faster because there is no testing for comments, delimiters, etc.

In conclusion, it seems to me that the use of streams and strings adds considerable overhead in all facets--they are inherently slower and in some ways their convenience becomes a hindrance in fundamental applications, include the '\r' not being handled.  I would think writing code with more basic and C-like constructs wouldn't be real difficult.  That would give the flexibility to check for '\r' new lines as well.

Dan Sebald <sebald>
Fri 01 Sep 2017 03:35:32 PM UTC, comment #26: 

Just got a report back from a question I asked to the Maintainer's list.  Matlab returns Inf for values which are out-of-range (e.g., 1e400) while Octave currently returns realmax (1.8e308).

So, back to the original poster's comment #14, it is in fact an improvement in compatibility that the current patch returns Inf for these cases.

Rik <rik5>
Group administrator
Thu 31 Aug 2017 08:59:53 PM UTC, comment #25: 

How many files have mixed EOL characters?  That seems like something way off on the tail of the distribution.  If a user has such a file they should probably just "fix" it using sed or perl before going any further.

But to see how Matlab behaves, I've attached a file which uses all three EOL markers.  It should read in as


[1 2 3
 4 5 6
 7 8 9
 1 2 3
 4 5 6
 7 8 9]


if Matlab is doing per-line EOL detection.


(file #41706)

Rik <rik5>
Group administrator
Thu 31 Aug 2017 08:39:02 PM UTC, comment #24: 

There is no guarantee that a file has consistent line endings, and as far as I know, Matlab and Octave both accept mixtures.  I think we should keep it that way.

OTOH, I can see that accepting any combination can cause trouble with reporting line numbers in error messages.

Maybe someone would like to test what Matlab actually does for mixed line endings in a data file?

John W. Eaton <jwe>
Group administrator
Thu 31 Aug 2017 08:28:12 PM UTC, comment #23: 

It would be nice if there were a way to open the file and communicate, at the time of opening, what the delimiter character was.  This would be an internal property associated with the stream object itself.  I took a look through some of the C++ documentation and I couldn't see that this was a possibility.  Of course, we could add this to a derived class, such as octave_stream.  But the current functions are using std::istream as an input and it would require switching to octave_stream through the call hierarchy.

Least good, but maybe okay, would be to have a static variable within get_mat_data_input_line () which records the delimiter.  For this to work we would need a way to identify when the std::istream input has changed.  Is there some unique identifier per stream?

Final thought, Do we need to strip all trailing '\r' or just the first one?  If we are on a Windows platform, and have used getline to capture the line and strip the '\n' character, then we only need to strip one '\r' character.  Any others were not part of the '\r\n' line ending and should probably stay.

Instead of this


// Remove tailing '\r'.
while (retval.size () && retval.back () == '\r')
  retval.pop_back ();


I think this would work


// Remove tailing '\r'.
if (! retval.empty () && retval.back () == '\r')
  retval.pop_back ();


This would allow lines where '\r' was legitamately in the text at the end of the line.

Rik <rik5>
Group administrator
Thu 31 Aug 2017 07:09:12 PM UTC, comment #22: 


>> The obvious test would be to peek into the first, say 255 bytes, of the file and see if we find a line ending.


What I suggested #21 is meant to find out that line ending.  Anyway, I should have pointed out in the previous post that my thinking was along the lines of sed, grep, wc (which can tell one the number of lines in a file).  That is, these character-by-character, causal, in-line operations that don't rewind file pointers or string pointers are all rather fast.

Dan Sebald <sebald>
Thu 31 Aug 2017 06:57:59 PM UTC, comment #21: 

Perhaps the easiest approach would be to avoid the standard functions like getline, strings, etc., and instead write a little C routine that

1) Identifies what the new-line character is
2) Determines what the longest line of the file is

With that information, we can pre-assign a buffer length in which to capture at least the longest line.

Something like (psuedo code):


char *newlineopts[] = {
  "\cr";
  "\cr\lf";
  "\r";
}

maxstrlen = 0;
thisstrlen = 0;
newlinestr = \null;
OPEN STREAM FOR CHARACTER READING
while (!oef) {
  GET CHAR cval FROM STREAM
  for (i=0; i<length(newlineopts); i++) {
    if (newlineopts[i][0] == cval) {
      if (newlineopts[i][1] == '\0') {
        newlinestr = newlineopts[i];
        maxstrlen = thisstrlen > maxstrlen ? thisstrlen : maxstrlen;
        thisstrlen = 0;
        continue;
      } else {
        GET CHAR cval FROM STREAM;
        if (newlineopts[i][1] == cval) {
          newlinestr = newlineopts[i];
          maxstrlen = thisstrlen > maxstrlen ? thisstrlen : maxstrlen;
          thisstrlen = 0;
          continue;
        }
      }
    }
  thisstrlen++;
}

// We now know the maximum line length in the file and what
// the EOL sequence is.
ASSIGN BUFFER linebuf OF SIZE maxstrlen+1.
REWIND FILE STREAM
while (!eof) {
  FILL linebuf UNTIL NEW LINE SEQUENCE FOUND
  PROCESS WITH THE CURRENT CODE
}


Something like that.  I'm just trying to make the point that we can probably create a custom "getline" that is very efficient without too much effort.

Dan Sebald <sebald>
Thu 31 Aug 2017 06:50:26 PM UTC, comment #20: 

Given that opening in binary and processing character by character is going to be slow, can we detect the file format before processing?  If so, the function std::getline has a form which uses whatever delimiter you like


istream& getline (istream& is, string& str, char delim);


This speed-up patch could then be only minimally modified.

The obvious test would be to peek into the first, say 255 bytes, of the file and see if we find a line ending.

Rik <rik5>
Group administrator
Thu 31 Aug 2017 06:44:56 PM UTC, comment #19: 

I reviewed the latest patch and updated it a bit.  See speed-up-load-ascii-v4.patch attached.

Mostly it was Octave coding conventions, but I did decide to use


return T (0.0);


from the template str2fp_num rather than just "return 0;".

I also used xelem rather than elem which will be slightly faster since it won't check that the row and column indices are valid on each access (we already know they are)


-              tmp.elem (i, j) = d;
+              tmp.xelem (i, j) = d;




(file #41705)

Rik <rik5>
Group administrator
Thu 31 Aug 2017 06:10:45 PM UTC, comment #18: 

The rest of Octave goes to great lengths to support text files with CR LF or CRLF line endings on any system, not just the native line ending for a given system, typically by opening all files as binary and liberally accepting any combination of CR and LF as the end of a line.  As far as I know, that's the way Matlab also behaves.  We have often received bug reports about files that can't be processed that have turned out to be due to getting this wrong.  It's confusing to users, because the tools they use to display files don't usually show these characters, so they don't know what the line ending characters are.  So I'd prefer to continue to liberally accept any combination of these characters to mean the end of a line.

John W. Eaton <jwe>
Group administrator
Thu 31 Aug 2017 06:02:49 PM UTC, comment #17: 

Item # 1: Will need to fix octave_read_fp_value in order for sscanf to pass

Item # 2: Comment processing should be kept.  First, because we need to be Matlab-compatible.  Second, because if you just have raw numbers in a file it can be super confusing.  It is far better to be able to add a few lines at the top of the file saying what the data is, when it was collected, what the format is, etc.

Item # 3: I don't think we need to support a bare '\r' line ending.  Looking at the Newline page on Wikipedia (https://en.wikipedia.org/wiki/Line_endings) there are only a few systems that use '\r' alone


CR:    Commodore 8-bit machines, Acorn BBC, ZX Spectrum, TRS-80, Apple II family, Oberon, the classic Mac OS, MIT Lisp Machine and OS-9


None of these are very popular anymore, and if necessary one can use Perl to transform the line endings before processing with Octave.

Rik <rik5>
Group administrator
Thu 31 Aug 2017 05:52:15 PM UTC, comment #16: 

Regarding delimiters, following link to &quot;load&quot;:  http://www.mathworks.com/help/matlab/ref/load.html#input_argument_d0e580248 .  Semicolon is in the list, and it does the same thing Octave is doing, i.e., accepting more variations on input characters than it outputs.

I've been updating from Mint 17.3 to Mint 18.2 over the past couple days (looks the same, but it's a change from gtk2 to gtk3...seems more robust so far).  So I can't try many changes just yet.  I'll just make some comments for now.

This hunk


+      // Remove tailing '\r'.
+      while (retval.size () &amp;&amp; retval.back () == '\r')
+        retval.pop_back ();


doesn't make much difference then, probably due to the fact retval is a string.  I was thinking that retval.size() is like strlen() in the sense it has to scan from the start of the string to find the '\0'.  However, I am guessing that strings compute their size when allocating memory for the string.  Thus, as Count points out the retval.size() and retval.back() are very efficient pointer manipulations.  Of course, all the overhead in terms of CPU is at the creation and initialization of the string.

Definitely we want to avoid this use of strings:


            std::string buf = get_mat_data_input_line (is);


because that get_mat_data_input_line() is creating a fresh string every time it is called.  I think Count is right that a malloc() is required for that.  That's too much system activity.  I would hope that this construct


+              std::getline (is, buf);


where buf is a string created only once, eventually only exercises malloc() if the string length needs to be increased, i.e., more space.

From #14:

&gt;&gt; With patch in #10, plus without get_lines_and_columns() (by hand input the nc and nr): 0.981622 sec.

That's a big difference.  It sure would be nice to eliminate that  2.3 s.  Rather than get the exact number of rows on the first pass, could we get the maximum number of rows (e.g., count the number of newline characters in the data with some fast C function) and then trim back the Matrix once we know the eventual size on the second pass which tosses out comment lines?

&gt;&gt; And no removal of any comment in get_mat_data_input_line(): 0.735889 sec.

That's a pretty sizable fraction too, once we first bring the benchmark down to the order of one second.  This hunk


+      // Remove any comment.
+      size_t pos_comment = retval.find_first_of (&quot;#%&quot;);
+      if (pos_comment != std::string::npos)
+        retval.erase (pos_comment);


is the one I was most concerned about because I know regardless of whether retval is a string or simple buffer this code has to scan through a whole line if there is no comment, i.e., the most probable scenario.  That is why I say it might be faster to scan for numeric values, and when that fails then check for comment characters.

This hunk


+      // Detect non-whitespace.
+      no_data_found = (retval.find_first_not_of (&quot; \t\r&quot;) == std::string::npos);


, however, is a negative search so will return as soon as it finds a numeric character.  So that's not CPU consuming (I may have been thinking otherwise previously).

BUT, isn't above a bug?  There are plenty of non-numeric characters that will cause no_data_found to be logical 0.  For example, the second line here


1.2 3.4 5.6
@ @ @
6.5 4.3 2.1


Do strings have some kind of member function similar to &quot;isnumeric()&quot;?  Say, retval.find_first_numeric()?  We wouldn't want to get such a long string as

retval.find_first_of (&quot;0123456789.-+&quot;)

would we?

There are other comments I could make, but let's first get the main bottlenecks out of the way.

Dan Sebald <sebald>
Thu 31 Aug 2017 04:53:35 PM UTC, comment #15: 

To summarize:

1) octave_read_fp_value() will still cause 2 fail assert on sscanf test. Need to fix this anyway.

2) Decide whether comments are allowed.
I personally like this feature because then it is convenient to tag a file, although seldom used. Matlab support comment here by using '%' but not '#' (I just tried), and run time is about 1.75 sec, we are not far behind.

3) All patches I provided here are lack the support to '\r' line ending, i.e. Mac OS Classic style, should we add it back?
Matlab support '\r' ending BTW.
std::getline is not able to detect both '\n' and '\r' line endings....

Eddy <count>
Thu 31 Aug 2017 12:02:28 AM UTC, comment #14: 


== Response to the NA test (#6 #7 #8 #10) ==

Converting a string to a double correctly is not an easy task, see here for a good explanation, you may also have a glance at the glibc strtod implementation. Thus some extra work to check the special value should not affect the speed. What surprised me is the high overhead of stream based IO operations (I optimized my own program then found that the bottleneck now on the Octave side, which lead to this bug report).

I made a mistake in octave_read_fp_value(), which compares std::string to char*. I have fix it in a newer patch based on the patch in #10.

The behaviour of octave_read_fp_value() is also tuned to match old one as possible, specially the fallbit and stream pointer. Exception:


Input                old                this patch
1e1000                1.79769e+308        +inf
-1e1000                1.79769e+308        -inf
0X1.23P+45        0                3.99947e+13


But the sscan will still fail, see the end.

== Response to #12 ==

The line returned by std::getline() is scanned twince, one for comments, one for convert the numbers. retval.back() and retval.pop_back() are fast. Not too slow I think.

Ideally, scan once is enough: get one char and if it is not comment, push_back to a std::string, then skip comment and '\n' or '\r' until reach a new line. This is how the old way works, the big drawback is that "is.get()" is incredibly slow compare to reading from a cache (e.g. buf[ptr++]).

Combining get_lines_and_columns () and get_mat_data_input_line () is a possible optimization, since that reduce "malloc" for std::string.



Let's review what can be speed up (measure by tic; b = load('dat.txt'); toc, baseline is 3.31289 sec):

With patch in #10, plus without get_lines_and_columns() (by hand input the nc and nr): 0.981622 sec.

And no removal of any comment in get_mat_data_input_line(): 0.735889 sec.

And put get_mat_data_input_line() into get_mat_data_input_line() that eliminate the use of std::sstream: 0.641812 sec.

The load-text mode is still faster for the same data set: 0.429227 sec. Note that load-text also get benefit from the new strtod implimentation. Speed of old one is: 0.582489 sec.

Look into load-text, the essential step is:


// libinterp/corefcn/load-save.cc (do_load) ->
// libinterp/corefcn/ls-oct-text.cc (read_text_data) ->
// libinterp/octave-value/ov-re-mat.cc (octave_matrix::load_ascii)
Matrix tmp (nr, nc);
is >> tmp;


But if I use it for get_mat_data_input_line(), the speed is slower somehow: 0.828483 sec.




The uploaded patch preserves get_lines_and_columns(), run time is about 1.04 second. Also a duplicated function for string to double conversion.

The failed tests are:


[val, count, msg, pos] = sscanf ("3I2", "%f");
ASSERT errors for:  assert (pos,2)

  Location  |  Observed  |  Expected  |  Reason
     ()           4            2         Abs err 2 exceeds tol 0

[val, count, msg, pos] = sscanf ("3In2", "%f");
!!!!! test failed
ASSERT errors for:  assert (pos,2)

  Location  |  Observed  |  Expected  |  Reason
     ()           5            2         Abs err 3 exceeds tol 0



(file #41703)

Eddy <count>
Wed 30 Aug 2017 05:38:36 PM UTC, comment #13: 

The use of a constant, ASCIIDELIM, might be nice practice.  However, I'm not sure it needs to contain anything other than space and tab.  See http://www.mathworks.com/help/matlab/ref/save.html where it appears that those are the only two delimiters allowed.

I don't think the scanning overhead is as large as you imply.  But there is a potential modification that might help.

As long as strtod ignores '\r' characters then we don't actually need to remove them from the string.  That would save a bit.  So I tried commenting out


+      // Remove tailing '\r'.
+      while (retval.size () && retval.back () == '\r')
+        retval.pop_back ();


and then adding '\r' to the list of characters to ignore


+      // Detect non-whitespace.
+      no_data_found = (retval.find_first_not_of (" \t\r") == std::string::npos);


There was some bouncing around in the benchmarks, but it seem to save just 10 milliseconds which on the benchmark I was using was 0.6%.  So, not a whole lot, but maybe worth it.

Big problem is still failing sscanf, dlmread, and importdata tests in BIST which are probably related to the position of the stream when a read fails.

Rik <rik5>
Group administrator
Wed 30 Aug 2017 02:22:22 AM UTC, comment #12: 

OK, I'm looking just at the patch.  This hunk


+      std::getline (is, retval);
+
+      // Remove tailing '\r'.
+      while (retval.size () && retval.back () == '\r')
+        retval.pop_back ();
+
+      // Remove any comment.
+      size_t pos_comment = retval.find_first_of ("#%");
+      if (pos_comment != std::string::npos)
+        retval.erase (pos_comment);
+
+      // Detect non-whitespace.
+      no_data_found = (retval.find_first_not_of (" \t") == std::string::npos);


seems rather wasteful in the sense that it is scanning a whole, possibly long line of ASCII characters multiple times to find what most likely is nothing.  It is like tripling the amount of scanning effort.  Does the comment character need to be the first character in a line for the line to be officially a comment line?  Why search through the whole line if numeric characters appear early in the line?

Generally for that main loop that calls this routine, couldn't this strategy be changed so that the priority is to scan the line for floats and if the scan fails then figure out what went wrong?  It would require a bit more processing mixed in with the loop, but it would be three or four times as efficient and more along the lines of the simpler functions that Count used.

For example, if the first float sscanf fails, then check if the first character is a "%#", if so call the line a comment and skip.  With that approach, does the tailing \r matter anymore?

I'd say, try figuring out what is wrong with dlmread.cc then commit the most recent changeset.  After that, try revamping this routine so that the testing is more integrated in the looping and rather than two functions

get_lines_and_columns ()
get_mat_data_input_line ()

combine their functionality into one and use no function calls.  Instead, if one doesn't want to use efficient array expansion, use something like (psuedo-code):


for (int i_scan = 0; i_scan < 2; i_scan++) {
    LOOP_TRHOUGH_ALL_LINES {
      if (i_scan == 1)
          tmp.elem (i, j) = d;
    }
    if (i_scan == 1)
        break;
    ASSIGN_MATRIX_MEMORY;
}


Does that sound like a more efficient approach?

Dan Sebald <sebald>
Wed 30 Aug 2017 01:44:42 AM UTC, comment #11: 

As far as I can tell, Octave documentation is lacking.

You mentioned delimiters.  Rather than use ", \t" in several locations, defining ASCIIDELIM or something similar would be better practice.  Semicolon should be accepted too.  Colon?

Dan Sebald <sebald>
Wed 30 Aug 2017 12:54:35 AM UTC, comment #10: 

I think accepting 'na' would just be nice.  If you're using a bare ASCII file for data transfer between various programs then it is possible that some other piece of SW wrote everything out in lower case.

As for accepting comments, what does Matlab do?  The existing Octave code that was rewritten accepted comments, so in order not to introduce a regression we should probably keep things the way they are.  But we could open a new bug report and discussion about removing that feature from the Octave load function.

I tested bypassing tolower and the savings were really minimal so I think it makes sense, for code clarity, to just use the library function.  But I changed tolower to toupper so that I can test for 'N' and 'A' which looks better than 'n' and 'a' given that this is how Octave writes the constant.

I also switched from using erase() to pop_back() since we are guaranteed C++11 now for Octave code.  I made other minor changes for clarity and to follow Octave coding conventions.

I'm seeing a 45% reduction in load times with this patch, call it about 1/2 the time it used to take.

I've attached the latest version of this cset to the bug report.  I can't commit it yet because it causes new failures in the test suite.  For example, 'test dlmread.cc' has errors.  This is probably something to do with how the stream positioning gets set.



(file #41696)

Rik <rik5>
Group administrator
Tue 29 Aug 2017 11:39:36 PM UTC, comment #9: 

A couple discussion points.

1) As far as 'na'/'Na'/'nA' vs. 'NA', why accept lower case letters if Octave doesn't accept them at the command line, e.g.,


octave:9> x = [na, 1, pi]
error: 'na' undefined near line 1 column 6


2) The help for 'load' states


     '-ascii'
          Force Octave to assume the file contains columns of numbers in
          text format without any header or other information.  Data in
          the file will be loaded as a single numeric matrix with the
          name of the variable derived from the name of the file.


which doesn't mention any elaborate handling of comment lines, etc.  The documentation and actual internal programming should match more closely whatever the behavior is.

Dan Sebald <sebald>
Tue 29 Aug 2017 11:28:18 PM UTC, comment #8: 

Good point, testing for 'N' before 'n' and 'A' before 'a' will likely save a few cycles for canonical files.  I was just thinking back on Intel x86 instructions, and it wouldn't surprise me if an instruction could be packed with 8 bits for op-code and 8 bits operand so that something like str_chunk[0] == 'N' could be very fast, and keeping operands in registers rather than swaping them out and back in is always good.  (A good optimizing compiler would figure that out.)

Of course, it all depends on how often this particular bit of code gets called, and there are likely other locations consuming enough cycles that such optimizing is irrelevant, so benchmark.  There have been similar Octave functions where successive tweaking has made a noticeable difference.

Dan Sebald <sebald>
Tue 29 Aug 2017 11:00:59 PM UTC, comment #7: 

I think it's a cute trick to avoid tolower, but the real test is just to benchmark it.  One's intuition about these things is often wrong. 

But before checking this, I would reorder things so that the first character checked is 'N' and in the next step 'A'.  If the value really is NA, this is the canonical form and it will shortcircuit faster.


octave:1> x = [NA, 1, pi]
x =

       NA   1.0000   3.1416

octave:2> save -ascii blah.txt x


And in blah.txt


 NA 1.00000000e+00 3.14159265e+00



Rik <rik5>
Group administrator
Tue 29 Aug 2017 09:52:05 PM UTC, comment #6: 

OK, nice work.  A bit much for me to follow right now, but I just wanted to note some coding nuances.  In this hunk:


+      // Error during conversion.
+      if (str_chunk.size () == 2 &&
+          std::tolower (str_chunk[0]) == 'n' &&
+          std::tolower (str_chunk[1]) == 'a')
+        {
+          // It is Octave NA.
+          val = octave::numeric_limits<T>::NA ();
+        }


would it be more efficient (since speed for large data is important) to not use std::tolower(), which I assume is a function call (is it a library function, or does the C++ header for the library know to use an inline?).  The function call will take extra time for the stack/jump/return.  The ::tolower has to do some tests for range and then subtract a value (unless it is going to use some internal 2^8-length lookup table for speed).

Plus, I'm wondering if the str_chunk.size() is extraneous because what affects the size value is str_chunk[0] == '\0', str_chunk[1] == '\0' or str_chunk[2] == '\0', which is effectively checked by testing str_chunk[0] being 'n' or 'N'.  In other words, the following is equivalent:


+      // Error during conversion.
+      if ((str_chunk[0]) == 'n' || str_chunk[0] == 'N')) &&
+          (str_chunk[1]) == 'a' || str_chunk[1] == 'A')) &&
+          str_chunk[2] == '\0')
+        {
+          // It is Octave NA.
+          val = octave::numeric_limits<T>::NA ();
+        }


Dan Sebald <sebald>
Tue 29 Aug 2017 09:00:38 PM UTC, comment #5: 

Yeah, some details are omitted, because I think operations on the matrix is negligible in this case.

Various improvements can be made:

Using the same benchmark as #0


tic; b = load('dat.txt'); toc


Original time (seconds)
3.30725

1) (Function get_mat_data_input_line) retval = &quot;&quot;; change to retval.erase ();
Run time: 3.25221

2) Reuse tmp_stream in function read_mat_ascii_data (and (1)).
Run time: 2.90416

3) Use std::getline in get_mat_data_input_line (and (1)(2))
Run time: 1.75087

4) Use strtod to do the convertion (and (1)(2)(3)).
Run time: 1.56847

See the patch.

Note that:

  3*) By using std::getline, I drop the support of '\r' line ending (Mac OS Classic).

  4*) By use strtod, I break the rule that stop reading at the position where it is not part of a number. This make Octave function such as sscanf() behave abnormally. This could be fixed by seekg() the stream, but octave_istrstream may not support this operation well.

(file #41695)

Eddy <count>
Tue 29 Aug 2017 08:59:20 PM UTC, comment #4: 

Just to give an idea of what is possible, I created the large variable 'a' and saved it both in an ascii file and in Octave's own text format with


save ('-text', 'odat.txt', 'a');


Benchmarking the read shows that ASCII files load ~4X slower than even regular text files.


tic; b = load ('dat.txt'); toc
Elapsed time is 4.86173 seconds.
clear b
tic; b = load ('odat.txt'); toc
Elapsed time is 1.13398 seconds.



Rik <rik5>
Group administrator
Tue 29 Aug 2017 07:46:19 PM UTC, comment #3: 

push_back of std::vector:  OK, that's good.  I would guess this is the case seeing the reduced time you cited.

I'm noting the extra step of:


Function read_mat_ascii_data()
{

  Function get_lines_and_columns()
  {
     Read the file line by line (get_mat_data_input_line()), exam number of columns, see if it consistent. The validation of numerics is disabled (check_numeric = false);
     Return number of columns and rows.
  }

  ASSIGN THE MEMORY FOR WHOLE NRxNC MATRIX.

  Read the file line by line (get_mat_data_input_line()), parse the numbers one by one in a line, and put it in the matrix.
  AND FILL IN USING tmp.elem (i, j) = d; WHERE tmp IS CLASS Matrix.
}

Function get_mat_data_input_line()
{
  Return a data line, skips comments and white or empty lines.
}


OK, from your guess, where is the most time used?  Is it determining number of rows and columns?  Is it getting the line, i.e., get_mat_data_input_line()? because it has to look for comments and empty lines?  Is it the next loop back from that?  In other words, what can be sped up here?

If it is determining number of rows and columns, then perhaps that could be skipped and use v.push_back().  If it is getting the line, then I'm not so sure.  If it is the loop that calls get_mat_data_input_line, then I'm not so sure either.  However, another strategy to keep in mind is that get_lines_and_columns() could also check if there are any comments, empty lines.  If not, then your fast reading approach could be used; otherwise fall back on the existing code.

Dan Sebald <sebald>
Tue 29 Aug 2017 06:58:26 PM UTC, comment #2: 

The push_back of std::vector does expansion in an efficient way, usually double or 1.5 times the capacity.

Octave did some error checking, but not much. This how it works:


Function read_mat_ascii_data()
{

  Function get_lines_and_columns()
  {
     Read the file line by line (get_mat_data_input_line()), exam number of columns, see if it consistent. The validation of numerics is disabled (check_numeric = false);
     Return number of columns and rows.
  }

  Read the file line by line (get_mat_data_input_line()), parse the numbers one by one in a line, and put it in the matrix.
}

Function get_mat_data_input_line()
{
  Return a data line, skips comments and white or empty lines.
}


Eddy <count>
Tue 29 Aug 2017 05:24:43 AM UTC, comment #1: 

Does

v.push_back(d);

expand the vector size in an efficient way?  E.g., does it grow the available space incrementally with each new addition to the vector?  Or does it double the size of available space when there is no more room in the vector?

Worth noting that in your test program the format of the data is assumed to be valid.  However, the Octave version may be looking at the data file in a more general sense to ensure that the format for each line has a certain consistency and sanity about it with respect to previous lines.

Definitely worth speeding up, if possible.  Would make a lot of users happy.

Dan Sebald <sebald>
Sun 27 Aug 2017 06:15:42 PM UTC, original submission:  

Demo of slowness:
(All tests are been run several times to ensure file is cached.)


a = 1e40 * rand(1e6,1);   % huge numbers can make strtod() a bit slower.
save('-ascii', '-double', 'dat.txt', 'a');

tic
b = load('dat.txt');
toc



Elapsed time is 3.3219 seconds.

But for binary read, it is much faster (x100 times).

+verbatim+
tic
fid = fopen('dat.txt');
c = fread(fid, Inf, '*char');
fclose(fid);
toc
-verbatim-

+nomarkup+ Elapsed time is 0.0309799 seconds.

To be more fair, here reads the numbers using C++ fstream.

+verbatim+
// g++ -O2 fstream_read_double.cpp && time ./a.out

#include <fstream>
#include <vector>

int main()
{
double d;
std::vector<double> v;
std::ifstream fin("dat.txt");
while (fin >> d) {
v.push_back(d);
}
return (int)v.size(); // Avoid optimize out v, if any.
}
-verbatim-

+nomarkup+ real 0m0.477s

Still an order of magnitude faster than Octave's load().

----

After test the source code, I found that most of the time is spent in

+verbatim+
// libinterp/ls-mat-ascii.cc

static std::string
get_mat_data_input_line (std::istream& is)
-verbatim-

The data operations of sstream and fstream has high overhead, especially sstream.

The C++ code piece above has this overhead for every *line* of input, but the get_mat_data_input_line() code has this overhead for every *character*!

I'm testing a speed up of the load-ascii code, hope that moderate the slowness.

Eddy <count>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #42492:  speed-up-load-ascii-v8.patch added by sebald (13KiB - text/x-patch)
file #42485:  speed-up-load-ascii-v6.patch added by sebald (12KiB - text/x-patch)
file #42487:  speed-up-load-ascii-v7.patch added by sebald (11KiB - text/x-patch)
file #42476:  ERANGE_tst.txt added by sebald (54B - text/plain)
file #42475:  speed-up-load-ascii-v5.patch added by sebald (11KiB - text/x-patch)
file #41706:  EOL_tst.txt added by rik5 (38B - text/plain)
file #41705:  speed-up-load-ascii-v4.patch added by rik5 (10KiB - text/x-patch)
file #41703:  speed-up-load-ascii-v3.patch added by count (10KiB - text/x-diff - Speed up experiment (#14).)
file #41696:  ascload.cset added by rik5 (7KiB - application/octet-stream)
file #41695:  speed-up-load-ascii-v1.patch added by count (7KiB - text/x-diff - Speed up experiment (#4).)

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by sebald (Posted a comment)
  • -email is unavailable- added by count (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 17 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-11-26 sebald Attached File- Added speed-up-load-ascii-v8.patch, #42492
    2017-11-25 sebald Attached File- Added speed-up-load-ascii-v6.patch, #42485
        Attached File- Added speed-up-load-ascii-v6v2.patch, #42486
        Attached File- Added speed-up-load-ascii-v7.patch, #42487
    2017-11-25 sebald Attached File- Added ERANGE_tst.txt, #42476
    2017-11-25 sebald Attached File- Added speed-up-load-ascii-v5.patch, #42475
    2017-11-06 rik5 Dependencies- bugs #52355 is dependent
    2017-09-08 sebald Attached File- Added octave-check_bad_and_eof_bit-djs2017sep08.patch, #41768
    2017-09-08 sebald Attached File- Added octave-check_bad_bit-djs2017sep08.patch, #41767
    2017-08-31 rik5 Attached File- Added EOL_tst.txt, #41706
    2017-08-31 rik5 Attached File- Added speed-up-load-ascii-v4.patch, #41705
    2017-08-31 count Attached File- Added speed-up-load-ascii-v3.patch, #41703
    2017-08-30 rik5 Attached File- Added ascload.cset, #41696
        StatusConfirmed In Progress
    2017-08-29 count Attached File- Added speed-up-load-ascii-v1.patch, #41695
    2017-08-29 rik5 StatusNone Confirmed
        Summaryload ascii-based numeric file is significantly slow loading '-ascii' format files is slow

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code