Patchwork [libfortran] PR 50016 Slow IO on Windows due to _commit()

login
register
mail settings
Submitter Janne Blomqvist
Date Oct. 31, 2011, 6:57 p.m.
Message ID <CAO9iq9EyujkDR0AUWHVUz0Tge6PTo++Rk7VrjM_BVz4iXEFauA@mail.gmail.com>
Download mbox | patch
Permalink /patch/122922/
State New
Headers show

Comments

Janne Blomqvist - Oct. 31, 2011, 6:57 p.m.
Hi,

here's an updated version of my patch that gets rid of _commit along
with a section in the manual describing data consistency and
durability issues.

See also the thread starting at

http://gcc.gnu.org/ml/fortran/2011-10/msg00079.html

and the latest mail in that thread with my current thinking which
perhaps explains some of the motivations behind this patch:

http://gcc.gnu.org/ml/fortran/2011-10/msg00141.html

Regtested on x86_64-unknown-linux-gnu, Ok for trunk?

frontend ChangeLog:

2011-10-31  Janne Blomqvist  <jb@gcc.gnu.org>

	PR libfortran/50016
	* gfortran.texi (Data consistency and durability): New section.


testsuite ChangeLog:

2011-10-31  Janne Blomqvist  <jb@gcc.gnu.org>

	PR libfortran/50016
	* gfortran.dg/inquire_size.f90: Don't flush the unit.

libgfortran ChangeLog:

2011-10-31  Janne Blomqvist  <jb@gcc.gnu.org>

	PR libfortran/50016
	* io/inquire.c (inquire_via_unit): Flush the unit and use ssize.
	* io/unix.c (buf_flush): Don't call _commit.
Janne Blomqvist - Nov. 7, 2011, 5:28 p.m.
PING!

On Mon, Oct 31, 2011 at 20:57, Janne Blomqvist
<blomqvist.janne@gmail.com> wrote:
> Hi,
>
> here's an updated version of my patch that gets rid of _commit along
> with a section in the manual describing data consistency and
> durability issues.
>
> See also the thread starting at
>
> http://gcc.gnu.org/ml/fortran/2011-10/msg00079.html
>
> and the latest mail in that thread with my current thinking which
> perhaps explains some of the motivations behind this patch:
>
> http://gcc.gnu.org/ml/fortran/2011-10/msg00141.html
>
> Regtested on x86_64-unknown-linux-gnu, Ok for trunk?
>
> frontend ChangeLog:
>
> 2011-10-31  Janne Blomqvist  <jb@gcc.gnu.org>
>
>        PR libfortran/50016
>        * gfortran.texi (Data consistency and durability): New section.
>
>
> testsuite ChangeLog:
>
> 2011-10-31  Janne Blomqvist  <jb@gcc.gnu.org>
>
>        PR libfortran/50016
>        * gfortran.dg/inquire_size.f90: Don't flush the unit.
>
> libgfortran ChangeLog:
>
> 2011-10-31  Janne Blomqvist  <jb@gcc.gnu.org>
>
>        PR libfortran/50016
>        * io/inquire.c (inquire_via_unit): Flush the unit and use ssize.
>        * io/unix.c (buf_flush): Don't call _commit.
>
>
> --
> Janne Blomqvist
>
jerry DeLisle - Nov. 9, 2011, 2:17 a.m.
On 11/07/2011 12:28 PM, Janne Blomqvist wrote:
> PING!
>
> On Mon, Oct 31, 2011 at 20:57, Janne Blomqvist
> <blomqvist.janne@gmail.com>  wrote:
>> Hi,
>>
>> here's an updated version of my patch that gets rid of _commit along
>> with a section in the manual describing data consistency and
>> durability issues.
>>
>> See also the thread starting at
>>
>> http://gcc.gnu.org/ml/fortran/2011-10/msg00079.html
>>
>> and the latest mail in that thread with my current thinking which
>> perhaps explains some of the motivations behind this patch:
>>
>> http://gcc.gnu.org/ml/fortran/2011-10/msg00141.html
>>
>> Regtested on x86_64-unknown-linux-gnu, Ok for trunk?
>>

OK, thanks Janne

Best regards,

Jerry

Patch

diff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi
index f847df3..b45b71a 100644
--- a/gcc/fortran/gfortran.texi
+++ b/gcc/fortran/gfortran.texi
@@ -1090,6 +1090,7 @@  might in some way or another become visible to the programmer.
 * KIND Type Parameters::
 * Internal representation of LOGICAL variables::
 * Thread-safety of the runtime library::
+* Data consistency and durability::
 @end menu
 
 
@@ -1194,6 +1195,81 @@  Finally, for platforms not supporting thread-safe POSIX functions,
 further functionality might not be thread-safe.  For details, please
 consult the documentation for your operating system.
 
+
+@node Data consistency and durability
+@section Data consistency and durability
+@cindex consistency, durability
+
+This section contains a brief overview of data and metadata
+consistency and durability issues when doing I/O.
+
+With respect to durability, GNU Fortran makes no effort to ensure that
+data is committed to stable storage. If this is required, the GNU
+Fortran programmer can use the intrinsic @code{FNUM} to retrieve the
+low level file descriptor corresponding to an open Fortran unit. Then,
+using e.g. the @code{ISO_C_BINDING} feature, one can call the
+underlying system call to flush dirty data to stable storage, such as
+@code{fsync} on POSIX, @code{_commit} on MingW, or @code{fcntl(fd,
+F_FULLSYNC, 0)} on Mac OS X. The following example shows how to call
+fsync:
+
+@smallexample
+  ! Declare the interface for POSIX fsync function
+  interface
+    function fsync (fd) bind(c,name="fsync")
+    use iso_c_binding, only: c_int
+      integer(c_int), value :: fd
+      integer(c_int) :: fsync
+    end function fsync
+  end interface
+
+  ! Variable declaration
+  integer :: ret
+
+  ! Opening unit 10
+  open (10,file="foo")
+
+  ! ...
+  ! Perform I/O on unit 10
+  ! ...
+
+  ! Flush and sync
+  flush(10)
+  ret = fsync(fnum(10))
+
+  ! Handle possible error
+  if (ret /= 0) stop "Error calling FSYNC"
+@end smallexample
+
+With respect to consistency, for regular files GNU Fortran uses
+buffered I/O in order to improve performance. This buffer is flushed
+automatically when full and in some other situations, e.g. when
+closing a unit. It can also be explicitly flushed with the
+@code{FLUSH} statement. Also, the buffering can be turned off with the
+@code{GFORTRAN_UNBUFFERED_ALL} and
+@code{GFORTRAN_UNBUFFERED_PRECONNECTED} environment variables. Special
+files, such as terminals and pipes, are always unbuffered. Sometimes,
+however, further things may need to be done in order to allow other
+processes to see data that GNU Fortran has written, as follows.
+
+The Windows platform supports a relaxed metadata consistency model,
+where file metadata is written to the directory lazily. This means
+that, for instance, the @code{dir} command can show a stale size for a
+file. One can force a directory metadata update by closing the unit,
+or by calling @code{_commit} on the file descriptor. Note, though,
+that @code{_commit} will force all dirty data to stable storage, which
+is often a very slow operation.
+
+The Network File System (NFS) implements a relaxed consistency model
+called open-to-close consistency. Closing a file forces dirty data and
+metadata to be flushed to the server, and opening a file forces the
+client to contact the server in order to revalidate cached
+data. @code{fsync} will also force a flush of dirty data and metadata
+to the server. Similar to @code{open} and @code{close}, acquiring and
+releasing @code{fcntl} file locks, if the server supports them, will
+also force cache validation and flushing dirty data and metadata.
+
+
 @c ---------------------------------------------------------------------
 @c Extensions
 @c ---------------------------------------------------------------------
diff --git a/gcc/testsuite/gfortran.dg/inquire_size.f90 b/gcc/testsuite/gfortran.dg/inquire_size.f90
index 568c3d6..13876cf 100644
--- a/gcc/testsuite/gfortran.dg/inquire_size.f90
+++ b/gcc/testsuite/gfortran.dg/inquire_size.f90
@@ -8,7 +8,9 @@  open(25, file="testfile", status="replace", access="stream", form="unformatted")
 do i=1,100
   write(25) i, "abcdefghijklmnopqrstuvwxyz"
 enddo
-flush(25)
+! Gfortran implicitly flushes the buffer when doing a file size
+! inquire on an open file.
+! flush(25)
 
 inquire(unit=25, named=is_named, name=aname, size=i)
 if (.not.is_named) call abort
diff --git a/libgcc/configure b/libgcc/configure
old mode 100644
new mode 100755
diff --git a/libgfortran/io/inquire.c b/libgfortran/io/inquire.c
index fb525ca..a542334 100644
--- a/libgfortran/io/inquire.c
+++ b/libgfortran/io/inquire.c
@@ -409,7 +409,10 @@  inquire_via_unit (st_parameter_inquire *iqp, gfc_unit * u)
 	  if (u == NULL)
 	    *iqp->size = -1;
 	  else
-	    *iqp->size = file_size (u->file, (gfc_charlen_type) u->file_len);
+	    {
+	      sflush (u->s);
+	      *iqp->size = ssize (u->s);
+	    }
 	}
     }
 
diff --git a/libgfortran/io/unix.c b/libgfortran/io/unix.c
index c87be13..f320733 100644
--- a/libgfortran/io/unix.c
+++ b/libgfortran/io/unix.c
@@ -451,10 +451,6 @@  buf_flush (unix_stream * s)
   if (s->ndirty != 0)
     return -1;
 
-#ifdef _WIN32
-  _commit (s->fd);
-#endif
-
   return 0;
 }