diff mbox

[Fortran,4.6,committed] PR 50016: mitigate performance regression on Windows by calling less often _commit

Message ID 4E9D783B.1050309@net-b.de
State New
Headers show

Commit Message

Tobias Burnus Oct. 18, 2011, 12:59 p.m. UTC 
This patch has been approved by Janne off list - and has been committed 
to the 4.6 branch only (Rev. 180138) after bootstrapping and regtesting it.

It is essentially my patch from
   http://gcc.gnu.org/ml/fortran/2011-10/msg00120.html
minus the .texi change. And the inquire.c part of Janne's patch at
   http://gcc.gnu.org/ml/fortran/2011-10/msg00094.html

In GCC 4.6 and 4.7, every time when the gfortran-internal buffer is 
flushed, _commit() was called on _WIN32 (= MinGW and MinGW-w64), which 
caused a severe slowdown. The reason is that _commit() causes that files 
are written to the hard disk.

With this patch, _commit() is only called when the user explicitly runs 
the FLUSH subroutine/statement. Additionally, if one inquires the size 
of an open file, now the internally known size is used instead of 
calling "stat".

  * * *

For 4.7 the same issue exists but as the release is still a couple of 
months away, we have time to learn more about how Windows handles 
buffering and which consistency should be provided. - The gist of the 
discussion is whether flush() should automatically call _commit or 
whether it shouldn't, which is a question about consistency vs. 
performance. For details, see the thread starting at 
http://gcc.gnu.org/ml/fortran/2011-10/threads.html#00079

For 4.6 we decided that it makes more sense to make the committed patch 
available for 4.6.2 than to delay it further. The patch should fix most 
of the performance issues.

Comment about which strategy to choose for 4.7 and insight about the 
buffering of Windows (i.e. whether it affects data and file meta data 
such as filesizes, or only the latter) are highly welcome.

Tobias
diff mbox

Patch

Index: libgfortran/ChangeLog
===================================================================
--- libgfortran/ChangeLog	(revision 180134)
+++ libgfortran/ChangeLog	(working copy)
@@ -1,3 +1,14 @@ 
+2011-10-18  Tobias Burnus  <burnus@net-b.de>
+	    Janne Blomqvist  <jb@gcc.gnu.org>
+
+	PR fortran/50016
+	* io/file_pos.c (st_flush): Call _commit on MinGW(-w64).
+	* io/intrinsics.c (flush_i4, flush_i8): Ditto.
+	* io/unix.c (flush_all_units_1, flush_all_units): Ditto.
+	(buf_flush): Remove _commit call.
+	* io/inquire.c (inquire_via_unit): Flush internal buffer
+	and call file_length instead of invoking stat via file_size.
+
 2011-09-11  Thomas Koenig  <tkoenig@gcc.gnu.org>
 
 	Backport fron trunk
Index: libgfortran/io/file_pos.c
===================================================================
--- libgfortran/io/file_pos.c	(revision 180134)
+++ libgfortran/io/file_pos.c	(working copy)
@@ -452,6 +452,10 @@  st_flush (st_parameter_filepos *fpp)
         fbuf_flush (u, u->mode);
 
       sflush (u->s);
+#ifdef _WIN32
+      /* Without _commit, changes are not visible to other file descriptors.  */
+      _commit (u->s->fd);
+#endif
       unlock_unit (u);
     }
   else
Index: libgfortran/io/inquire.c
===================================================================
--- libgfortran/io/inquire.c	(revision 180134)
+++ libgfortran/io/inquire.c	(working copy)
@@ -409,7 +409,10 @@  inquire_via_unit (st_parameter_inquire *iqp, gfc_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 = file_length (u->s);
+	    }
 	}
     }
 
Index: libgfortran/io/unix.c
===================================================================
--- libgfortran/io/unix.c	(revision 180134)
+++ libgfortran/io/unix.c	(working copy)
@@ -435,10 +435,6 @@  buf_flush (unix_stream * s)
   if (s->ndirty != 0)
     return -1;
 
-#ifdef _WIN32
-  _commit (s->fd);
-#endif
-
   return 0;
 }
 
@@ -1542,7 +1538,14 @@  flush_all_units_1 (gfc_unit *u, int min_unit)
 	  if (__gthread_mutex_trylock (&u->lock))
 	    return u;
 	  if (u->s)
-	    sflush (u->s);
+	    {
+	      sflush (u->s);
+#ifdef _WIN32
+	      /* Without _commit, changes are not visible to other
+		 file descriptors.  */
+	      _commit (u->s->fd);
+#endif
+	    }
 	  __gthread_mutex_unlock (&u->lock);
 	}
       u = u->right;
@@ -1573,6 +1576,11 @@  flush_all_units (void)
       if (u->closed == 0)
 	{
 	  sflush (u->s);
+#ifdef _WIN32
+	  /* Without _commit, changes are not visible to other
+	     file descriptors.  */
+	  _commit (u->s->fd);
+#endif
 	  __gthread_mutex_lock (&unit_lock);
 	  __gthread_mutex_unlock (&u->lock);
 	  (void) predec_waiting_locked (u);
Index: libgfortran/io/intrinsics.c
===================================================================
--- libgfortran/io/intrinsics.c	(revision 180134)
+++ libgfortran/io/intrinsics.c	(working copy)
@@ -207,6 +207,11 @@  flush_i4 (GFC_INTEGER_4 *unit)
       if (us != NULL)
 	{
 	  sflush (us->s);
+#ifdef _WIN32
+	  /* Without _commit, changes are not visible
+	     to other file descriptors.  */
+	  _commit (u->s->fd);
+#endif
 	  unlock_unit (us);
 	}
     }
@@ -230,6 +235,11 @@  flush_i8 (GFC_INTEGER_8 *unit)
       if (us != NULL)
 	{
 	  sflush (us->s);
+#ifdef _WIN32
+	  /* Without _commit, changes are not visible
+	     to other file descriptors.  */
+	  _commit (u->s->fd);
+#endif
 	  unlock_unit (us);
 	}
     }