Re: Performance optimization in av::fixup - use buffered IO, not mapped file

[Daniel Colascione <dancol at dancol dot org>]

On 12/11/2012 5:06 PM, Daniel Colascione wrote:
> On 12/10/2012 7:51 PM, Daniel Colascione wrote:
>> The key to generating a binary that repros the problem is to unexec emacs, then
>> try to repro with that generated binary, not a copy of it.
> 
> The real explanation is a lot simpler: the binary is sparse. When you create a
> file mapping object for a sparse file, Windows discards all cached pages for
> that file. It makes sense that compilers (and Emacs unexec) would create sparse
> files as they seek around inside their outputs.

Anyway, the binary is sparse because our linker produces sparse files.

Would the Cygwin developers accept this patch? With it, applications would need
to explicitly use ftruncate to make files sparse. Considering the horrible and
unexpected performance implications of sparse files, I don't think generating
them automatically from a sequence of seeks and writes is the right thing to do.


Index: fhandler.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/fhandler.cc,v
retrieving revision 1.429
diff -u -r1.429 fhandler.cc
--- fhandler.cc	16 Aug 2012 23:34:43 -0000	1.429
+++ fhandler.cc	12 Dec 2012 03:10:31 -0000
@@ -821,32 +821,6 @@
   FILE_POSITION_INFORMATION fpi;
   FILE_STANDARD_INFORMATION fsi;

-  if (did_lseek ())
-    {
-      did_lseek (false); /* don't do it again */
-
-      if (!(get_flags () & O_APPEND)
-	  && NT_SUCCESS (NtQueryInformationFile (get_output_handle (),
-						 &io, &fsi, sizeof fsi,
-						 FileStandardInformation))
-	  && NT_SUCCESS (NtQueryInformationFile (get_output_handle (),
-						 &io, &fpi, sizeof fpi,
-						 FilePositionInformation))
-	  && fpi.CurrentByteOffset.QuadPart
-	     >= fsi.EndOfFile.QuadPart + (128 * 1024)
-	  && (pc.fs_flags () & FILE_SUPPORTS_SPARSE_FILES))
-	{
-	  /* If the file system supports sparse files and the application
-	     is writing after a long seek beyond EOF, convert the file to
-	     a sparse file. */
-	  NTSTATUS status;
-	  status = NtFsControlFile (get_output_handle (), NULL, NULL, NULL,
-				    &io, FSCTL_SET_SPARSE, NULL, 0, NULL, 0);
-	  debug_printf ("%p = NtFsControlFile(%S, FSCTL_SET_SPARSE)",
-			status, pc.get_nt_native_path ());
-	}
-    }
-
   if (wbinary ())
     res = raw_write (ptr, len);
   else
@@ -1069,10 +1043,6 @@
     }
   _off64_t res = fpi.CurrentByteOffset.QuadPart;

-  /* When next we write(), we will check to see if *this* seek went beyond
-     the end of the file and if so, potentially sparsify the file. */
-  did_lseek (true);
-
   /* If this was a SEEK_CUR with offset 0, we still might have
      readahead that we have to take into account when calculating
      the actual position for the application.  */
Index: fhandler.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/fhandler.h,v
retrieving revision 1.475
diff -u -r1.475 fhandler.h
--- fhandler.h	13 Oct 2012 12:34:17 -0000	1.475
+++ fhandler.h	12 Dec 2012 03:10:31 -0000
@@ -138,10 +138,6 @@
     unsigned wbinary		: 1; /* binary write mode */
     unsigned wbinset		: 1; /* binary write mode explicitly set */
     unsigned nohandle		: 1; /* No handle associated with fhandler. */
-    unsigned did_lseek		: 1; /* set when lseek is called as a flag that
-					_write should check if we've moved
-					beyond EOF, zero filling or making
-					file sparse if so. */
     unsigned query_open		: 3; /* open file without requesting either
 					read or write access */
     unsigned close_on_exec      : 1; /* close-on-exec */
@@ -151,7 +147,7 @@
    public:
     status_flags () :
       rbinary (0), rbinset (0), wbinary (0), wbinset (0), nohandle (0),
-      did_lseek (0), query_open (no_query), close_on_exec (0),
+      query_open (no_query), close_on_exec (0),
       need_fork_fixup (0), isclosed (0)
       {}
   } status, open_status;
@@ -243,7 +239,6 @@
   IMPLEMENT_STATUS_FLAG (bool, wbinset)
   IMPLEMENT_STATUS_FLAG (bool, rbinset)
   IMPLEMENT_STATUS_FLAG (bool, nohandle)
-  IMPLEMENT_STATUS_FLAG (bool, did_lseek)
   IMPLEMENT_STATUS_FLAG (query_state, query_open)
   IMPLEMENT_STATUS_FLAG (bool, close_on_exec)
   IMPLEMENT_STATUS_FLAG (bool, need_fork_fixup)

Attachment: signature.asc
Description: OpenPGP digital signature