231300 - nsLocalFile::MoveTo is very slow to move directories [Cause of disk trashing (on NT) when clearing cache?]

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P2)

Description

[Jo Hermans]

Build 2004011605 on Windows NT

I noticed that clearing the disk cache was a lot faster on Mac OS X 10.2.8
(about a second), than on Windows NT (can take up to half a minute or more).
When investigating why, I found that all cache-files are moved to Cache.Trash,
and are then deleted in a background thread. The deletion itself is more than
fast enough (esp. because it's on a separate thread), but it's the move that is
the bottleneck. You can hear the disk trashing like crazy. Hence the name of the
folder, I suppose :-)

Mac OS X handles the move of an entire directoy very fast (see
nsLocalFile::MoveToNative, which ends up calling nsLocalFile::MoveCopy at
<http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsLocalFileOSX.cpp#1917>), but
the same code in Windows is a mess (see nsLocalFile::MoveToNative, which ends up
calling nsLocalFile::CopySingleFile at
<http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsLocalFileWin.cpp#980>). From
what I can see, the code is creating a backupfile first (.moztmp extension),
before the real MoveFile is called. Isn't that a bit too much ? Shouldn't
nsLocalfile::CopyMove call MoveFile directly, instead of passing through
CopySingleFile ? And can we avoid the backupfile in this case ?

See also bug 179641.

(filed in Browser/Xpcom, just like bug 179641)

Comment 1

[Doug Turner (:dougt)]

have you measured this or is this speculation?

creating the moztmp files is to prevent data loss when a move fails.  osX may do
this for you auto-magically.

Comment 2

[Jo Hermans]

I found the code while I was on IRC at home (on Mac OS X). Tomorrow, in the
office, I'll try to see if I can find it back on Windows NT.

The difference between clearing the disk cache on Mac OS X and Windows NT is
enormous : the Mac does it in 1 or 2 seconds (even on an old 300 MHz iBook),
while Windows will keep trashing the disk for half a minute or more (400 MHz
desktop). It's very audible and blocks the GUI for a long time.

Comment 3

[Jo Hermans]

Note that I can't see the problem anymore on Windows 2000, on exactly the same
hardware (and same software - currently Firefox 0.9.1). I think that the
filesystem code in Windooze has improved a lot. Those .moztmp files are only
created for a very short time, so they might never be written to disk.

Oh well, if nobody has any complaints about it, we can close this bug (and
179641 too). It might still be very inefficient code (we're deleting files, so
those backup files are overkill), but the OS can easily help in this case.

Note that you also are deleting the entire cache after a crash. On my old NT
setup, I also had to wait +/- 30 sec before the browser loaded, both in Mozilla
and Firefox.

Comment 4

[timeless]

the cache deletion is intentional/by design. cache does not do any work to
maintain its integrity in the case when the app crashes, so its only recovery
mechanism is to start from scratch.

Comment 5

[Jo Hermans]

(In reply to comment #4)
> the cache deletion is intentional/by design. cache does not do any work to
> maintain its integrity in the case when the app crashes, so its only recovery
> mechanism is to start from scratch.

I know, you have to read the other comments too. I was only commenting that
because of the slow delete (b/c every delete creates a backup file first !), it
is slower than it needs to be. I don't have anything against the delete itself.

Comment 6

[OstGote!]

see also new bug 287827 (dupe?)

Comment 7

[Darin Fisher]

*** Bug 287827 has been marked as a duplicate of this bug. ***

Comment 8

[Darin Fisher]

Bug 287827 has more analysis on this problem.  It is very clear that iterating
the directory and moving individual files is killing performance.  We should
only do that if MoveFile fails (due to crossing filesystem volumes or something
like that).

Comment 9

[Daniel Kirsch]

I create authoring applications for web based trainings using the Mozilla
Framework. Such a project could have a lot of media files and pages. More then
1000 Files with more than 500 MB in total are pretty common.
When I want to just rename a folder it takes up to several minutes as the whole
folder will be moved instead of just renamed.
As a nsIFile.moveTo(null,newName) seems to be the only way to rename a file or
directory (using JavaScript), this is pretty unsatisfying.
Hope to see this fixed in one of the next releases.

Comment 10

[Darin Fisher]

adding to my 1.8 bucket... help would still be very much appreciated!

Comment 11

[Alfred Kayser]

From comparing:
http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsLocalFileWin.cpp#1427
with:
http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsLocalFileUnix.cpp#861
one can see that the Unix version of MoveToNative 
first just tries to 'rename' the file or directory, whilst the Win version
always CopyMove's it.

Copying the code from the Unix version could do the trick, replacing 'rename'
with the windows 'MoveFile' call.

P.s. same applies also to OS/2:
http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsLocalFileOS2.cpp#800

Comment 12

[Darin Fisher]

Yeah, I agree.  That sounds like the right solution.  Do you have time to put
together a patch?

Comment 13

[Alfred Kayser]

Attachment: Version 1: patch for Windows and OS/2

Instead of always enumerating directories to move it, first try to move
the directory as a single 'file' to the new location. If this fails, fallback
to the usual iterative move/copy.
This speeds up the move/delete operation of a dirty cache enormously, as the
move
is now a single file op, and delete is a separate nonblocking thread.

This patch also does the same trick of OS/2, but I cannot compile and test
OS/2.

Any volunteers do the testing for this patch?

Comment 14

[Darin Fisher]

Comment on attachment 181040 [details] [diff] [review]
Version 1: patch for Windows and OS/2

This patch looks like it should do the trick (and I'll help test it), but it's
interesting that it may call CopySingleFile more than once in the failure case
for normal files.

Comment 15

[Alfred Kayser]

Attachment: Version 2: Improved logic for trying CopySingleFile for copy file and/or move file/directories

This version only tries to call CopySingleFile for copy of single files (or
non-followed symlinks), and for move of a single file or a directory. 
Only when the CopySingleFile for move of a directory fails, fallback to the
enumeration code (which is also used for normal copy of directories and
followed symlinks)).

Comment 16

[Darin Fisher]

Comment on attachment 181068 [details] [diff] [review]
Version 2: Improved logic for trying CopySingleFile for copy file and/or move file/directories

r=darin

Comment 17

[Doug Turner (:dougt)]

Comment on attachment 181068 [details] [diff] [review]
Version 2: Improved logic for trying CopySingleFile for copy file and/or move file/directories

xpcom/io only needs your review darin.

Since I am here:

the comment:
// Ignore failure in case of move of a directory, then we fallback to
enumeration

Doesn't read well, try:
// If we are moving a directory and that fails, fallback on directory
enumberation.  See bug 231300 for details.

Also, if you really wanted to you could make CopySingleFile not make a backup
while doing the move.  Probably just removing that will will you a ~40% speed
inprovement over what we do currently.	However, doing this will result in data
loss if the move files for whatever reason.  Maybe there isn't ever a failure
case we care about?

Comment 18

[Alfred Kayser]

I would be carefull about removing the backup because MoveFile is mostly used
for things like cookies.txt, prefs.js, bookmarks.html. Losing these files in
case of failure is not so nice...

Comment 19

[Alfred Kayser]

Actually the renamve, movefile, delete operation could also be replaced by
'ReplaceFile' (see bug 179641). Problem is that the HandleFile function is only
supported on W2K and WXP (and higher I guess).
Note that on OS/2 we don't create a backup first, so failure is failure there...

Comment 20

[Darin Fisher]

The backup business in CopySingleFile is only done for single files.  Moreover,
it is only done in the case where we are moving a file on top of an existing
file.  So, for directories there is nothing to worry about.

Comment 21

[Darin Fisher]

Comment on attachment 181068 [details] [diff] [review]
Version 2: Improved logic for trying CopySingleFile for copy file and/or move file/directories

I like dougt's suggested wording of that comment -- without the 'b' of course
;-)

I'd like to get some testing of this fix in 1.8b2.  It solves the very real
problem of firefox thrashing the harddrive madly after a firefox or OS crash.

Comment 22

[Asa Dotzler [:asa]]

Comment on attachment 181068 [details] [diff] [review]
Version 2: Improved logic for trying CopySingleFile for copy file and/or move file/directories

a=asa

Comment 23

[Darin Fisher]

fixed-on-trunk

thanks again Alfred!

Comment 24

[Christian :Biesinger (don't email me, ping me on IRC)]

*** Bug 300035 has been marked as a duplicate of this bug. ***