From patchwork Tue Oct 18 12:59:39 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tobias Burnus X-Patchwork-Id: 120420 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 75039B6FF5 for ; Wed, 19 Oct 2011 00:00:06 +1100 (EST) Received: (qmail 25625 invoked by alias); 18 Oct 2011 12:59:59 -0000 Received: (qmail 25610 invoked by uid 22791); 18 Oct 2011 12:59:57 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE X-Spam-Check-By: sourceware.org Received: from mx01.qsc.de (HELO mx01.qsc.de) (213.148.129.14) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 18 Oct 2011 12:59:42 +0000 Received: from [192.168.178.22] (port-92-204-119-220.dynamic.qsc.de [92.204.119.220]) by mx01.qsc.de (Postfix) with ESMTP id 8A3893CDA3; Tue, 18 Oct 2011 14:59:40 +0200 (CEST) Message-ID: <4E9D783B.1050309@net-b.de> Date: Tue, 18 Oct 2011 14:59:39 +0200 From: Tobias Burnus User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:7.0.1) Gecko/20110929 Thunderbird/7.0.1 MIME-Version: 1.0 To: gfortran , gcc patches Subject: [Patch, Fortran, 4.6, committed] PR 50016: mitigate performance regression on Windows by calling less often _commit Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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 + Janne Blomqvist + + 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 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); } }