Re: [patch, fortran] Large transfer support for IO library

[Thomas Koenig <Thomas dot Koenig at online dot de>]

On Wed, Oct 05, 2005 at 10:51:49AM +0300, Janne Blomqvist wrote:

> To reiterate why IMHO this should be included
> 
> 1) It enables > 2 GB array transfers. Technically, this is a
> regression in the array IO functionality wrt the previous behavior,
> although no PR has been filed.
> 
> 2) It improves performance and reduces memory consumption for big
> transfers by avoiding buffering if the transfer size is > BUFFER_SIZE.
> 
> 3) There is already some slight bitrot wrt other patches, although to
> my knowledge it is at the moment in sync with cvs. E.g. Jakub:s recent
> thread safety patch, and indeed my own fix for 24174 (only attached to
> bugzilla, since I had mail problems when I finished the patch..).

I concur.

> +  if (ioparm.size != NULL)
> +    *ioparm.size += (GFC_INTEGER_4) nread;

This shows we still have an issue with very large transfers
and the size= parameter, which should have its own PR.

> +  if (nread != *nbytes)
> +    {				/* Short read, this shouldn't happen.  */

Well it can (and you are testing for a legitimate reason just below :-)

> +      /* Requests between SSIZE_MAX and SIZE_MAX are undefined by SUS,

What's SUS?  (Also below).

> +static try
>  fd_seek (unix_stream * s, gfc_offset offset)
>  {
> +  if (s->physical_offset == offset) /* Are we lucky and avoid syscall?  */
> +    {
> +      s->logical_offset = offset;
> +      return SUCCESS;
> +    }

Good catch!

>  /*********************************************************************
>    memory stream functions - These are used for internal files
>  
> @@ -641,6 +794,60 @@ mem_alloc_w_at (unix_stream * s, int *le
>  }
>  
>  
> +/* Stream read function for internal units. This is not actually used
> +   at the moment, as all internal IO is formatted and the formatted IO
> +   routines use mem_alloc_r_at.  */

Do you actually plan to use these?  Otherwise, you could just replace
their bodies with a tail call to gcc_unreachable().

This is OK for mainline.  Because this is a regression fix, I think
that this is also OK for commit at the moment.  I'd suggest waiting
for a day or so to see if people agree here.

	Thomas