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

[Corinna Vinschen <corinna-cygwin at cygwin dot com>]

On Dec 13 11:39, Corinna Vinschen wrote:
> On Dec 13 01:44, Daniel Colascione wrote:
> > On 12/13/12 1:09 AM, Corinna Vinschen wrote:
> > > Cygwin is used a lot for development, and it is famed for its slowness.
> > > If we can notably speed up normal operation by disabling the automatic
> > > sparse handling in lseek/write, I think we should do it.  For those
> > > requiring sparseness on NTFS, we can add a "sparse" mount option and
> > > run the code in lseek/write only if that mount flag is set.  And
> > > for symmetry we should probbaly do the same in ftruncate.
> > 
> > What about using the automatic sparse handling in lseek/lwrite and
> > ftruncate only when the file being operated on is already sparse?
> 
> That doesn't make sense.  If the file is already sparse, there's no
> reason to set the sparse flag in write or ftruncate again.  Also, if you
> set the sparse flag only on already sparse files, you will never be able
> to create sparse files.

Below is my cut on the issue.  It introduces a "sparse" mount option and
sparsifies a file only if this mount option is set.  I also slightly
improved the code in fhandler_base::write and ftruncate so that it will
not try to set the sparse flag on already sparse files.

The code compiles, so it's basically correct.  It's just not tested. ;)


Corinna


	* fhandler.cc (fhandler_base::write): Don't attempt to sparsify
	an already sparse file.  Drop check for FILE_SUPPORTS_SPARSE_FILES
	flag.  Explicitely set FILE_ATTRIBUTE_SPARSE_FILE attribute in
	cached attributes.
	(fhandler_base::lseek): Only set did_lseek if sparseness is supported. 
	* fhandler_disk_file.cc (fhandler_disk_file::ftruncate): Don't attempt
	to sparsify an already sparse file.  Explicitely set
	FILE_ATTRIBUTE_SPARSE_FILE attribute in cached attributes.
	* mount.cc (oopt): Add "sparse" flag.
	(fillout_mntent): Ditto.
	* path.h (enum path_types): Add PATH_SPARSE.
	(path_conv::support_sparse): New method.
	(path_conv::fs_flags): Constify.
	(path_conv::fs_name_len): Ditto.
	include/sys/mount.h: Replace unused MOUNT_MIXED flag with MOUNT_SPARSE.


Index: fhandler.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/fhandler.cc,v
retrieving revision 1.429
diff -u -p -r1.429 fhandler.cc
--- fhandler.cc	16 Aug 2012 23:34:43 -0000	1.429
+++ fhandler.cc	13 Dec 2012 14:29:35 -0000
@@ -817,15 +817,17 @@ ssize_t __stdcall
 fhandler_base::write (const void *ptr, size_t len)
 {
   int res;
-  IO_STATUS_BLOCK io;
-  FILE_POSITION_INFORMATION fpi;
-  FILE_STANDARD_INFORMATION fsi;
 
   if (did_lseek ())
     {
+      IO_STATUS_BLOCK io;
+      FILE_POSITION_INFORMATION fpi;
+      FILE_STANDARD_INFORMATION fsi;
+
       did_lseek (false); /* don't do it again */
 
       if (!(get_flags () & O_APPEND)
+	  && !has_attribute (FILE_ATTRIBUTE_SPARSE_FILE)
 	  && NT_SUCCESS (NtQueryInformationFile (get_output_handle (),
 						 &io, &fsi, sizeof fsi,
 						 FileStandardInformation))
@@ -833,8 +835,7 @@ fhandler_base::write (const void *ptr, s
 						 &io, &fpi, sizeof fpi,
 						 FilePositionInformation))
 	  && fpi.CurrentByteOffset.QuadPart
-	     >= fsi.EndOfFile.QuadPart + (128 * 1024)
-	  && (pc.fs_flags () & FILE_SUPPORTS_SPARSE_FILES))
+	     >= fsi.EndOfFile.QuadPart + (128 * 1024))
 	{
 	  /* If the file system supports sparse files and the application
 	     is writing after a long seek beyond EOF, convert the file to
@@ -842,6 +843,9 @@ fhandler_base::write (const void *ptr, s
 	  NTSTATUS status;
 	  status = NtFsControlFile (get_output_handle (), NULL, NULL, NULL,
 				    &io, FSCTL_SET_SPARSE, NULL, 0, NULL, 0);
+	  if (NT_SUCCESS (status))
+	    pc.file_attributes (pc.file_attributes ()
+				| FILE_ATTRIBUTE_SPARSE_FILE);
 	  debug_printf ("%p = NtFsControlFile(%S, FSCTL_SET_SPARSE)",
 			status, pc.get_nt_native_path ());
 	}
@@ -1071,7 +1075,8 @@ fhandler_base::lseek (_off64_t offset, i
 
   /* 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 (pc.support_sparse ())
+    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
Index: fhandler_disk_file.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/fhandler_disk_file.cc,v
retrieving revision 1.380
diff -u -p -r1.380 fhandler_disk_file.cc
--- fhandler_disk_file.cc	31 Oct 2012 15:02:13 -0000	1.380
+++ fhandler_disk_file.cc	13 Dec 2012 14:29:36 -0000
@@ -1189,12 +1189,15 @@ fhandler_disk_file::ftruncate (_off64_t 
       feofi.EndOfFile.QuadPart = length;
       /* Create sparse files only when called through ftruncate, not when
 	 called through posix_fallocate. */
-      if (allow_truncate
-	  && (pc.fs_flags () & FILE_SUPPORTS_SPARSE_FILES)
+      if (allow_truncate && pc.support_sparse ()
+	  && !has_attribute (FILE_ATTRIBUTE_SPARSE_FILE)
 	  && length >= fsi.EndOfFile.QuadPart + (128 * 1024))
 	{
 	  status = NtFsControlFile (get_handle (), NULL, NULL, NULL, &io,
 				    FSCTL_SET_SPARSE, NULL, 0, NULL, 0);
+	  if (NT_SUCCESS (status))
+	    pc.file_attributes (pc.file_attributes ()
+				| FILE_ATTRIBUTE_SPARSE_FILE);
 	  syscall_printf ("%p = NtFsControlFile(%S, FSCTL_SET_SPARSE)",
 			  status, pc.get_nt_native_path ());
 	}
Index: mount.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/mount.cc,v
retrieving revision 1.95
diff -u -p -r1.95 mount.cc
--- mount.cc	16 Aug 2012 23:34:44 -0000	1.95
+++ mount.cc	13 Dec 2012 14:29:36 -0000
@@ -1028,6 +1028,7 @@ struct opt
   {"override", MOUNT_OVERRIDE, 0},
   {"posix=0", MOUNT_NOPOSIX, 0},
   {"posix=1", MOUNT_NOPOSIX, 1},
+  {"sparse", MOUNT_SPARSE, 0},
   {"text", MOUNT_BINARY, 1},
   {"user", MOUNT_SYSTEM, 1}
 };
@@ -1667,6 +1668,9 @@ fillout_mntent (const char *native_path,
   if (flags & MOUNT_NOPOSIX)
     strcat (_my_tls.locals.mnt_opts, (char *) ",posix=0");
 
+  if (!(flags & MOUNT_SPARSE))		/* user mount */
+    strcat (_my_tls.locals.mnt_opts, (char *) ",sparse");
+
   if (!(flags & MOUNT_SYSTEM))		/* user mount */
     strcat (_my_tls.locals.mnt_opts, (char *) ",user");
 
Index: path.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/path.h,v
retrieving revision 1.171
diff -u -p -r1.171 path.h
--- path.h	31 Jul 2012 19:36:16 -0000	1.171
+++ path.h	13 Dec 2012 14:29:36 -0000
@@ -70,6 +70,7 @@ enum path_types
   PATH_EXEC		= MOUNT_EXEC,
   PATH_NOTEXEC		= MOUNT_NOTEXEC,
   PATH_CYGWIN_EXEC	= MOUNT_CYGWIN_EXEC,
+  PATH_SPARSE		= MOUNT_SPARSE,
   PATH_RO		= MOUNT_RO,
   PATH_NOACL		= MOUNT_NOACL,
   PATH_NOPOSIX		= MOUNT_NOPOSIX,
@@ -153,6 +154,11 @@ class path_conv
   bool has_acls () const {return !(path_flags & PATH_NOACL) && fs.has_acls (); }
   bool hasgood_inode () const {return !(path_flags & PATH_IHASH); }
   bool isgood_inode (__ino64_t ino) const;
+  bool support_sparse () const
+  {
+    return (path_flags & PATH_SPARSE)
+	   && (fs_flags () & FILE_SUPPORTS_SPARSE_FILES);
+  }
   int has_symlinks () const {return path_flags & PATH_HAS_SYMLINKS;}
   int has_dos_filenames_only () const {return path_flags & PATH_DOS;}
   int has_buggy_open () const {return fs.has_buggy_open ();}
@@ -342,8 +348,8 @@ class path_conv
   short get_unitn () const {return dev.get_minor ();}
   DWORD file_attributes () const {return fileattr;}
   void file_attributes (DWORD new_attr) {fileattr = new_attr;}
-  DWORD fs_flags () {return fs.flags ();}
-  DWORD fs_name_len () {return fs.name_len ();}
+  DWORD fs_flags () const {return fs.flags ();}
+  DWORD fs_name_len () const {return fs.name_len ();}
   bool fs_got_fs () const { return fs.got_fs (); }
   bool fs_is_fat () const {return fs.is_fat ();}
   bool fs_is_ntfs () const {return fs.is_ntfs ();}
Index: include/sys/mount.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/include/sys/mount.h,v
retrieving revision 1.15
diff -u -p -r1.15 mount.h
--- include/sys/mount.h	11 Aug 2010 10:58:06 -0000	1.15
+++ include/sys/mount.h	13 Dec 2012 14:29:36 -0000
@@ -26,8 +26,8 @@ enum
 					   device mount */
   MOUNT_CYGWIN_EXEC =	0x00040,	/* file or directory is or contains a
 					   cygwin executable */
-  MOUNT_MIXED	=	0x00080,	/* reads are text, writes are binary
-					   not yet implemented */
+  MOUNT_SPARSE	=	0x00080,	/* Support automatic sparsifying of
+					   files. */
   MOUNT_NOTEXEC =	0x00100,	/* don't check files for executable magic */
   MOUNT_DEVFS =		0x00200,	/* /device "filesystem" */
   MOUNT_PROC =		0x00400,	/* /proc "filesystem" */


-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat