[PATCH] Vastly improve GnomeVFS xfer performance

[Christian Neumair <chris gnome-de org>]

The attached patch vastly improves the Xfer perfomance of GnomeVFS, in
particular the sftp performance.

Changes

* When copying file data, consider both the source and the target I/O
block size, reading with source block-sized buffer, writing with target
block-sized buffer. It could be further refined with odd I/O block sizes
(cf. the FIXME comment).

* Increase sftp I/O block size to the number of max. concurrent requests
multiplied by the buffer size to exploit its parallelization caps. Also
make the code more readable and correct, by removing the odd (and buggy)
out_of_order semantics. Looks like somebody tried to optimize walking
through an array when network I/O is involved. *shrug*

Related bug report

http://bugzilla.gnome.org/show_bug.cgi?id=155872


-- 
Christian Neumair <chris gnome-de org>

Index: libgnomevfs/gnome-vfs-xfer.c
===================================================================
RCS file: /cvs/gnome/gnome-vfs/libgnomevfs/gnome-vfs-xfer.c,v
retrieving revision 1.135
diff -u -p -r1.135 gnome-vfs-xfer.c
--- libgnomevfs/gnome-vfs-xfer.c	3 Mar 2006 16:09:31 -0000	1.135
+++ libgnomevfs/gnome-vfs-xfer.c	12 Mar 2006 14:31:12 -0000
@@ -1204,11 +1204,13 @@ copy_file_data (GnomeVFSHandle *target_h
 		GnomeVFSProgressCallbackState *progress,
 		GnomeVFSXferOptions xfer_options,
 		GnomeVFSXferErrorMode *error_mode,
-		guint block_size,
+		guint source_block_size,
+		guint target_block_size,
 		gboolean *skip)
 {
 	GnomeVFSResult result;
 	gpointer buffer;
+	char *read_buffer;
 	const char *write_buffer;
 	GnomeVFSFileSize total_bytes_read;
 	gboolean forget_cache;
@@ -1219,16 +1221,19 @@ copy_file_data (GnomeVFSHandle *target_h
 		return GNOME_VFS_ERROR_INTERRUPTED;
 	}
 
-	buffer = g_malloc (block_size);
+	buffer = g_malloc (MAX (source_block_size, target_block_size));
 	total_bytes_read = 0;
 
 	/* Enable streaming if the total size is large */
 	forget_cache = progress->progress_info->bytes_total >= DROP_CACHE_SIZE_LIMIT;
 	
 	do {
+		GnomeVFSFileSize bytes_to_read;
 		GnomeVFSFileSize bytes_read;
+		GnomeVFSFileSize bytes_read_one;
 		GnomeVFSFileSize bytes_to_write;
 		GnomeVFSFileSize bytes_written;
+		GnomeVFSFileSize bytes_written_one;
 		gboolean retry;
 
 		progress->progress_info->status = GNOME_VFS_XFER_PROGRESS_STATUS_OK;
@@ -1236,43 +1241,65 @@ copy_file_data (GnomeVFSHandle *target_h
 
 		progress->progress_info->phase = GNOME_VFS_XFER_PHASE_READSOURCE;
 
+		read_buffer = buffer;
+
+		bytes_to_read = MAX (source_block_size, target_block_size);
+		bytes_read = 0;
+
 		do {
 			retry = FALSE;
 
-			result = gnome_vfs_read (source_handle, buffer,
-						 block_size, &bytes_read);
+			result = gnome_vfs_read (source_handle, read_buffer,
+						 MIN (bytes_to_read, source_block_size),
+						 &bytes_read_one);
 			if (forget_cache) {
 				gnome_vfs_forget_cache (source_handle,
-							total_bytes_read, bytes_read);
+							total_bytes_read, bytes_read_one);
 			}
 			if (result != GNOME_VFS_OK && result != GNOME_VFS_ERROR_EOF)
 				retry = handle_error (&result, progress,
 						      error_mode, skip);
-		} while (retry);
+
+			bytes_to_read -= bytes_read_one;
+			read_buffer += bytes_read_one;
+			bytes_read += bytes_read_one;
+		} while ((result == GNOME_VFS_OK && bytes_to_read > 0) || retry);
+
+		if (result == GNOME_VFS_ERROR_EOF && bytes_read > 0) {
+			/* process data accumulated in previous runs, before the EOF occurred */
+			result = GNOME_VFS_OK;
+		}
 
 		if (result != GNOME_VFS_OK || bytes_read == 0 || *skip) {
 			break;
 		}
 
 		total_bytes_read += bytes_read;
-		bytes_to_write = bytes_read;
 
 		progress->progress_info->phase = GNOME_VFS_XFER_PHASE_WRITETARGET;
 
+		bytes_to_write = bytes_read;
+		bytes_written = 0;
+
+		/* FIXME if source_block_size is bigger than target_block_size but not
+		 * a multiple, sometimes bytes_to_write will be smaller than the target
+		 * block size. We may want to store the rest for the next loop pass
+		 * in a temporary buffer. */
 		write_buffer = buffer;
 		do {
 			retry = FALSE;
 
 			result = gnome_vfs_write (target_handle, write_buffer,
-						  bytes_to_write,
-						  &bytes_written);
+						  MIN (bytes_to_write, target_block_size),
+						  &bytes_written_one);
 
 			if (result != GNOME_VFS_OK) {
 				retry = handle_error (&result, progress, error_mode, skip);
 			}
 
-			bytes_to_write -= bytes_written;
-			write_buffer += bytes_written;
+			bytes_to_write -= bytes_written_one;
+			write_buffer += bytes_written_one;
+			bytes_written += bytes_written_one;
 		} while ((result == GNOME_VFS_OK && bytes_to_write > 0) || retry);
 		
 		if (forget_cache && bytes_to_write == 0) {
@@ -1280,6 +1307,11 @@ copy_file_data (GnomeVFSHandle *target_h
 						total_bytes_read - bytes_read, bytes_read);
 		}
 
+		if (result == GNOME_VFS_OK && bytes_read != bytes_written) {
+			g_warning ("bytes read/bytes written don't match!");
+			result = GNOME_VFS_ERROR_INTERNAL;
+		}
+
 		progress->progress_info->phase = GNOME_VFS_XFER_PHASE_COPYING;
 
 		progress->progress_info->bytes_copied += bytes_read;
@@ -1443,7 +1475,8 @@ copy_symlink (GnomeVFSURI *source_uri,
 }
 
 static GnomeVFSResult
-copy_file (GnomeVFSFileInfo *info,  
+copy_file (GnomeVFSFileInfo *info,
+	   GnomeVFSFileInfo *target_dir_info,
 	   GnomeVFSURI *source_uri,
 	   GnomeVFSURI *target_uri,
 	   GnomeVFSXferOptions xfer_options,
@@ -1498,6 +1531,8 @@ copy_file (GnomeVFSFileInfo *info,  
 					  */
 					 (info->valid_fields & GNOME_VFS_FILE_INFO_FIELDS_IO_BLOCK_SIZE && info->io_block_size > 0)
 					 ? info->io_block_size : 8192,
+					 (target_dir_info->valid_fields & GNOME_VFS_FILE_INFO_FIELDS_IO_BLOCK_SIZE && target_dir_info->io_block_size > 0)
+					 ? target_dir_info->io_block_size : 8192,
 					 skip);
 	}
 
@@ -1644,7 +1679,8 @@ copy_directory (GnomeVFSFileInfo *source
 				progress_set_source_target_uris (progress, source_uri, dest_uri);
 
 				if (info->type == GNOME_VFS_FILE_TYPE_REGULAR) {
-					result = copy_file (info, source_uri, dest_uri, 
+					result = copy_file (info, target_dir_info,
+							    source_uri, dest_uri, 
 							    xfer_options, error_mode, overwrite_mode, 
 							    progress, &skip_child);
 				} else if (info->type == GNOME_VFS_FILE_TYPE_DIRECTORY) {
@@ -1657,7 +1693,8 @@ copy_directory (GnomeVFSFileInfo *source
 						result = gnome_vfs_get_file_info_uri (source_uri, symlink_target_info,
 										      GNOME_VFS_FILE_INFO_FOLLOW_LINKS);
 						if (result == GNOME_VFS_OK) 
-							result = copy_file (symlink_target_info, source_uri, dest_uri, 
+							result = copy_file (symlink_target_info, target_dir_info,
+									    source_uri, dest_uri, 
 									    xfer_options, error_mode, overwrite_mode, 
 									    progress, &skip_child);
 						gnome_vfs_file_info_unref (symlink_target_info);
@@ -1786,7 +1823,6 @@ copy_items (const GList *source_uri_list
 			if (result == GNOME_VFS_OK && GNOME_VFS_FILE_INFO_SGID(target_dir_info)) {
 				info->gid = target_dir_info->gid;
 			}
-			gnome_vfs_file_info_unref (target_dir_info);
 			result = GNOME_VFS_OK;
 
 			/* optionally keep trying until we hit a unique target name */
@@ -1811,7 +1847,8 @@ copy_items (const GList *source_uri_list
 				
 				
 				if (info->type == GNOME_VFS_FILE_TYPE_REGULAR) {
-					result = copy_file (info, source_uri, target_uri, 
+					result = copy_file (info, target_dir_info,
+							    source_uri, target_uri, 
 							    xfer_options, error_mode, 
 							    &overwrite_mode_abort, 
 							    progress, &skip);
@@ -1871,6 +1908,8 @@ copy_items (const GList *source_uri_list
 				
 				/* try again with new uri */
 			}
+
+			gnome_vfs_file_info_unref (target_dir_info);
 		}
 
 		gnome_vfs_file_info_unref (info);
Index: modules/sftp-method.c
===================================================================
RCS file: /cvs/gnome/gnome-vfs/modules/sftp-method.c,v
retrieving revision 1.40
diff -u -p -r1.40 sftp-method.c
--- modules/sftp-method.c	23 Feb 2006 19:08:12 -0000	1.40
+++ modules/sftp-method.c	12 Mar 2006 14:31:28 -0000
@@ -523,7 +523,7 @@ buffer_read_file_info (Buffer *buf, Gnom
 	}
 
 	info->valid_fields |= GNOME_VFS_FILE_INFO_FIELDS_IO_BLOCK_SIZE;
-	info->io_block_size = default_req_len;
+	info->io_block_size = max_req * default_req_len;
 
 	DEBUG4 (g_log (G_LOG_DOMAIN, G_LOG_LEVEL_DEBUG, "Read file info from %p", buf));
 }
@@ -1963,11 +1963,10 @@ do_read (GnomeVFSMethod       *method, 
 	Buffer msg;
 	char type;
 	int recv_id, status;
-	guint num_req, req_ptr, req_svc_ptr, req_svc;
+	guint num_req, req_ptr, req_svc;
 	guint len;
 	guchar *buffer;
 	guchar *curr_ptr;
-	gboolean out_of_order;
 	GnomeVFSResult result;
 	gboolean got_eof;
 	guint outstanding;
@@ -1989,7 +1988,6 @@ do_read (GnomeVFSMethod       *method, 
 	*bytes_read = 0;
 	num_req = 0;
 	req_ptr = 0;
-	req_svc_ptr = 0;
 	curr_ptr = buffer_in;
 	buffer = buffer_in;
 	outstanding = 0;
@@ -2041,16 +2039,12 @@ do_read (GnomeVFSMethod       *method, 
 		recv_id = buffer_read_gint32 (&msg);
 
 		/* Look for the id received among sent ids */
-		out_of_order = FALSE;
-		req_svc = req_svc_ptr;
-		while (read_req[req_svc].id != recv_id && req_svc != req_ptr) {
-			if (read_req[req_svc].id != 0)
-				out_of_order = TRUE;
-			if (++req_svc >= max_req)
-				req_svc = 0;
+		for (req_svc = 0; req_svc < max_req; req_svc++) {
+			if (read_req[req_svc].id == recv_id)
+				break;
 		}
 
-		if (req_svc == req_ptr) { /* Didn't find the id -- unexpected reply */
+		if (req_svc >= max_req) { /* Didn't find the id -- unexpected reply */
 			DEBUG (g_log (G_LOG_DOMAIN, G_LOG_LEVEL_DEBUG, "%s: Unexpected id %d",
 				      __FUNCTION__, recv_id));
 			buffer_free (&msg);
@@ -2119,9 +2113,6 @@ do_read (GnomeVFSMethod       *method, 
 			sftp_connection_unlock (handle->connection);
 			return GNOME_VFS_ERROR_PROTOCOL_ERROR;
 		}
-
-		if (!out_of_order)
-			req_svc_ptr = req_svc;
 	}
 
 	handle->offset += *bytes_read;