nsLocalFileMac::MoveCopy() turns around and calls the MoreFiles routine FSpFileCopy() to copy files. Morefiles uses a 16K buffer if you don't give it one. This makes copying large files (several megabytes) very slow, for example at the end of a big download. This would be easily fixed by passing in a bigger buffer to FSpFileCopy.
shouldn't we tell the finder to do this for big files? i'm certain they've optimized their code for doing all this.
AppleScripting the Finder would be ugly. I have a patch.
A 16K buffer!?!? No wonder. 2 megs seems excessive though. Did you run some tests and find out the optimum buffer size? A few years ago, I did some tests with I/O buffer sizes and found that the perf/buffer size curve starts flattening out after about 256K. Even so, the gain to justify 2 megs instead of 256K would have to be huge.
I did no analysis. Maybe someone could come up with some JS to call this so that I don't have to keep downloading files :)
Actually, this could be fixed much easier : go to http://lxr.mozilla.org/seamonkey/source/lib/mac/MoreFiles/FileCopy.c#51 , and change bigCopyBuffSize to 0x00040000, 256K (it's now 0x00004000, 16K) I remember this piece of sample code from Apple back from the 80-ies. The 16K was the magical number both for floppy drives and for (serial) LocalTalk. It's time to bump this up to a more normal value, like 64K or 256K. 2 megs don't make any sense at all, you'll block too long waiting for all the buffers in the OS. My own apps (Unix-based though) use 64K btw, but that's because they works across congested networks. 'Zilla should normally work on local hard disks or at worst on local networks. and the OS should deal with this in the background anyway. I can't build on my Mac, can someone test this ? And I think it's a no brainer to include it in the 0.9.5 release. You might have trouble with a 2 MB buffer if you're out of memory, but a 256K buffer should be fine. Anyway, when you can't allocate, you'll use the default 16K or even the minimal 0.5K, so this should be perfectly fine.
BTW, I've noticed that the Mac was pretty slow after downloading a large document, with lot of (unoptimised) disk activity. I think this could be the fix.
I made a little console test program which called FSpFileCopy with various buffer sizes and printed the results. The "Buffer size" field is the number which was passed to FSpFileCopy, "0" was really 16 KB. Buffer size = 0, KB/sec = 1330 Buffer size = 32768, KB/sec = 1734 Buffer size = 65536, KB/sec = 3192 Buffer size = 131072, KB/sec = 4198 Buffer size = 262144, KB/sec = 6474 Buffer size = 524288, KB/sec = 7444 Buffer size = 1048576, KB/sec = 8245 Buffer size = 2097152, KB/sec = 8840 You can see that, after 1 MB for the buffer, there's very little gain. I'd still say 256 KB is fine.
Target Milestone: --- → mozilla0.9.7
Target Milestone: mozilla0.9.7 → mozilla0.9.8
Created attachment 64909 [details] [diff] [review] Allocated a temp 512K handle for the copy buffer, and also use it for directory copies This patch tries to allocate a 512k handle in temporary memory to act as the copy buffer. This same buffer is used in the directory copy case as well now. Reviews please.
Attachment #52119 - Attachment is obsolete: true
Comment on attachment 64909 [details] [diff] [review] Allocated a temp 512K handle for the copy buffer, and also use it for directory copies r=sdagley (mmmm, faster)
Attachment #64909 - Flags: review+
Comment on attachment 64909 [details] [diff] [review] Allocated a temp 512K handle for the copy buffer, and also use it for directory copies sr=sspitzer
Attachment #64909 - Flags: superreview+
oops, r= in haste, need size of buffer passed to copy func
Created attachment 64928 [details] [diff] [review] Fixed patch, pass in buffer size.
Attachment #64909 - Attachment is obsolete: true
Created attachment 64930 [details] [diff] [review] 3rd time luck. If allocation fails, be sure to pass 0 as the buffer size.
Attachment #64928 - Attachment is obsolete: true
17 years ago
Attachment #64930 - Flags: superreview+
Comment on attachment 64930 [details] [diff] [review] 3rd time luck. If allocation fails, be sure to pass 0 as the buffer size. sr=sspitzer
Comment on attachment 64930 [details] [diff] [review] 3rd time luck. If allocation fails, be sure to pass 0 as the buffer size. r=sdagley (yep, 3rd time is the charm :-)
Attachment #64930 - Flags: review+
It's in! Thanks guys.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.