optimizations are much needed in the FTP filesystem

[Denis-de-Bernardy]

I added some extra tracking in the class to understand why it was timing out on a customer's server. the last message is what verbose mode outputs:

exists(/home/dietp4fn/public_html/)
is_file(/)
exists(/)
is_dir(/)
is_file(/)
exists(/)
is_dir(/)
is_dir(/wp-includes)
exists(/home/dietp4fn/public_html/wp-content)
is_file(/)
exists(/)
is_dir(/)
Found /wp-content

I suspect that is_file(), is_dir(), and exists() could all use a cache, and that is_dir() could use an optimization along the lines of:

if ( preg_match("/\.(php|css|js|txt|html|je?pg|png|gif|po|pot|mo)$/i", $file )
  return true;

[dd32]

Heres some code i use for debugging things (PHP5 only), requires you to rename the class you're debugging (dont forget the construct) but works nicely.

class WP_Filesystem_FTPext {
	var $a;
	var $handle;
	function __construct($args) {
		$this->a = new _WP_Filesystem_FTPext($args);
		$this->handle = fopen(ABSPATH . 'debug.txt', 'w+');
	}
	function __call($name, $args) {
		fwrite($this->handle, 'Calling ' . $name . '(' . implode(', ', $args) . ') = ');
		$res = call_user_func_array( array(&$this->a, $name), $args);
		if ( is_bool($res) )
			$hres = '(bool)' . ($res ? '1' : '0');
		else
			$hres = $res;
		fwrite($this->handle, '<pre>' . htmlentities(print_r($hres, true)) . "\r\n\r\n\r\n</pre>");
		return $res;
	}
	function __get($name) {
		return $this->a->$name;
	}
	function __set($name, $value) {
		return ($this->a->$name = $value);
	}
}

You're right, some of the methods could definately use a cache, But sometimes the cache needs to be erased.

I'd also suggest you try using trunk, i think theres been a few optimizations to certain parts..

[Denis-de-Bernardy]

cross-referencing a few notes I added to another ticket:

​http://core.trac.wordpress.org/ticket/10611#comment:6

[Denis-de-Bernardy]

see also #10403

[dd32]

This is on the books to be looked at after #10779, which will reduce some of the calls.

if ( preg_match("/\.(php|css|js|txt|html|je?pg|png|gif|po|pot|mo)$/i", $file )
  return true;

::is_file() is used to determine if its a file, and it exists, i believe, not just if the path looks like a file.

[Denis-de-Bernardy]

Replying to dd32:

::is_file() is used to determine if its a file, and it exists, i believe, not just if the path looks like a file.

Correct. We'd need to make sure the file exists. To do that, though, we might as well use the local file system, no matter which FS we use.

[dd32]

(In [12998]) Reduce code duplication, Formatting cleanups, Reduce Chmod IO calls. See #10913 for excess IO calls.

[aldenta]

Pass known information to copy and delete to save lookup operations

[dd32]

attachment 10913.diff added

Have been talking with aldenta about this this evening, The changes in the patch are very basic and simple, but decrease the amount of FTP operations needed by a huge margin. This also caused a site which previously took >5 minutes to complete sub-30seconds.
There are likely some modifications which can be made to the ftpsockets class as well to speed things up.

Note: The changes here only affect the FTPExt class, the Direct, FTPSockets and SSH classes would need to be updated to support the same set of arguments.

[stevem]

I have a number of blogs running on a server with WHM and Pure-FTPd which seems to be the type most affected. Automatic updates hadn't worked since upgrading to version 3.0.

I have now tested the suggested changes.

In order to update from 3.0.4 to 3.0.5 I changed just two files, wp-admin/includes/file.php and wp-admin/includes/class-wp-filesystem-ftpext.php according to ​10913.diff, uploaded them and the automatic update ran smoothly and quickly (sub-30 seconds).

[dd32]

I have now tested the suggested changes.

Thanks for testing!

I'll be getting these changes in as soon as 3.1 is out the door for 3.2, I'll probably bundle the changes into a Plugin as well to help out users for the duration of 3.1

[stevem]

More good news :) Uploading just those two changed files again (wp-admin/includes/file.php and wp-admin/includes/class-wp-filesystem-ftpext.php) allowed the update to 3.1 to proceed correctly and quickly.

[westi]

Plugin: ​http://wordpress.org/extend/plugins/ftp-upgrade-fix/ by @aldenta.

Bumping priority on this to ensure we don't miss including it in 3.2

[dd32]

just while I remember this, The other thing I played around with here was changing copy_dir() to use ::move() instead of ::copy(), which reduces the amount of read/writes (both of IO and reading into PHP Memory). Played with, but didn't benchmark.

[dd32]

(In [17525]) Optimisations to WP_Filesystem; Pass known information to called functions. Props aldenta (John Ford) for investigation and patch. See #10913

[dd32]

Just a note, [17525] is not entirely the same as 10913.diff, I removed a few extra calls and patched a few functions to behave like the rest (direct::copy for example).

[westi]

Dion - can we close this ticket now?

[Denis-de-Bernardy]

@Nacin: I haven't tested, and I may have missed a commit here or there, but I'm not entirely convinced this is fixed. Based on the two commits in this ticket, it seems to me that we'd still end up with things like:

is_file(/)
exists(/)
is_dir(/)
is_file(/)
exists(/)
is_dir(/)

[dd32]

Denis: We'll probably end up with that in a few locations still, however, this dramatically reduces the number of them, While benchmarking data isn't on this ticket, in some bad cases it reduced it from ~10,000 operations to less than 5,000 operations, many of which were duplicated is_file() & is_dir() calls.

Without adding an entire Filesystem caching layer which takes into account files being unlink'd and whatnot, this is the best that we'll have in the short term. (so I agree that this ticket should be closed)

[Denis-de-Bernardy]

Replying to dd32:

Without adding an entire Filesystem caching layer which takes into account files being unlink'd and whatnot.

... which is incidentally what the ticket was about... :-)

I'm seeing there's a filesystem_method_file filter though, so I'll be able to extend the needed classes if I ever feel like giving it a shot using a plugin...