This is the mail archive of the
fortran@gcc.gnu.org
mailing list for the GNU Fortran project.
[Patch, Fortran, 4.6, committed] PR 50016: mitigate performance regression on Windows by calling less often _commit
- From: Tobias Burnus <burnus at net-b dot de>
- To: gfortran <fortran at gcc dot gnu dot org>, gcc patches <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 18 Oct 2011 14:59:39 +0200
- Subject: [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);
}
}