[patch, libfortran] [4.7/4.8/4.9 Regression] PR38199 missed optimization: I/O performance

[Tobias Burnus]

Jerry DeLisle wrote:
> The attached patch addresses the problem identified in comment #22 of the PR.
> For character array internal unit reads, eat_spaces must call next_char to
> advance every single character until the end of the string is reached.  In the
> case sited which is very contrived, this amounts to about 100000 calls to next_char.

> Without the patch takes about 25 seconds to run.
> With the patch this takes about 2.8 seconds.

Nice!

> The speedup is accomplished by simply skipping over spaces without calling
> next_read, then backing up one character and letting the existing execution path
> proceed, preserving all the end of record code needed in next_char.

I think that code is okay.

I wonder whether it can happen that we read one character too far: i.e. 
stell() == end of string; offset++ -> one behind. As one then 
immediately steps back (due to "offset < limit" plus offset--), the code 
should be handled just fine - except for the out of bounds read - which 
might be detected by, e.g., -fsanitizer=address (if libgfortran and the 
program use it). However, I am not sure whether eat_spaces would be 
called in that case or whether other conditions error out before.

> I also remove some unneeded error checks.

I have to admit that I do not really understand why the conditions are 
unreachable. Is it because "dtp->u.p.current_unit->bytes_left == 0" is 
already checked for "is_array_io (dtp)"? If so, can it happen for scalar 
internal I/O? I assume, it is obvious, but I don't quickly see it.

Tobias


> 2014-03-08  Jerry DeLisle  <jvdelisle@gcc.gnu>
>
> 	PR libfortran/38199
> 	* io/list_read.c (next_char): Delete unuseful error checks.
> 	(eat_spaces): For character array reading, skip ahead over
> 	spaces rather than call next_char multiple times.