Simon Fraser [no longer active] Assignee
	Description • 23 years ago

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.

	Mike Pinkerton (not reading bugmail)
	Comment 1 • 23 years ago

shouldn't we tell the finder to do this for big files? i'm certain they've
optimized their code for doing all this.

	Simon Fraser [no longer active] Assignee
	Comment 2 • 23 years ago

AppleScripting the Finder would be ugly. I have a patch.

	Conrad Carlen (not reading bugmail)
	Comment 5 • 23 years ago

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.

	Simon Fraser [no longer active] Assignee
	Comment 6 • 23 years ago

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  :)

	Jo Hermans
	Comment 7 • 23 years ago

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.

	Jo Hermans
	Comment 8 • 23 years ago

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.

	Conrad Carlen (not reading bugmail)
	Comment 9 • 23 years ago

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.

	Simon Fraser [no longer active] Assignee
	Comment 10 • 23 years ago

0.9.7

	Simon Fraser [no longer active] Assignee
	Comment 11 • 23 years ago

->0.9.8

	Simon Fraser [no longer active] Assignee
	Comment 12 • 23 years ago

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.

	Steve Dagley
	Comment 13 • 23 years ago

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)

	(not reading, please use seth@sspitzer.org instead)
	Comment 14 • 23 years ago

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

	Steve Dagley
	Comment 15 • 23 years ago

oops, r= in haste, need size of buffer passed to copy func

	(not reading, please use seth@sspitzer.org instead)
	Comment 18 • 23 years ago

Comment on attachment 64930 [details] [diff] [review]
3rd time luck. If allocation fails, be sure to pass 0 as the buffer size.

sr=sspitzer

	Steve Dagley
	Comment 19 • 23 years ago

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 :-)

	Simon Fraser [no longer active] Assignee
	Comment 20 • 23 years ago

It's in! Thanks guys.