This is the mail archive of the fortran@gcc.gnu.org mailing list for the GNU Fortran project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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
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);
 	}
     }

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]