Two small fixes
Alan.Bateman at oracle.com
Mon Aug 23 07:36:59 PDT 2010
Rémi Forax wrote:
> a minor nit:
> line 468 (end of transferToTrustedChannel) should be :
> return count - remaining;
> instead of
> return count-remaining;
Thanks Rémi, I'll fix that and refresh the webrev.
> I wonder if the 8 megs is enough. I've played some time ago with
> Windows XP
> mmap using Java API and found that it was more efficient to use read()
> if the file length was less than few hundred megabytes.
> It was ok for linux, and i have not tested with more recent version of
This is definitely worth looking at. Memory mapping is usually a win on
Solaris/Linux although it might be more expensive for very small
transfers. Solaris cp(1), for example, only maps if the file is larger
than 32k. Windows is different and I haven't observed any real benefit
on various editions of Windows, even with large files. There probably
isn't one answer that is right for all platforms and we might have to
change FileChannel.transferXXX so that the transfer method uses the
right transfer strategy for the platform. So if it's okay with you, I'd
prefer to keep the current bugID focused on removing mapping of the
entire file and use a different bugID to "tune" the transfer methods.
One thing I should say is the most important case for the transferTo
method is where the target is a SocketChannel. Solaris/Linux will uses
sendfile as before and not the code in this patch. The 8MB limit
shouldn't be an issue on Windows because the socket buffer will limit
the size anyway. The changes in the patch really only impact the
FileChannel to FileChannel case when the transfer size is >8MB. I've run
tests with transfer sizes of 16, 32, 64, 128, 512MB... and I don't any
real difference compared to jdk7-b106 (this includes Windows XP and
Windows Server 2008).
> fine, even if the patch is named ScatterGather :)
Yeah, that's webrev as it names the patch based on the repo name and I
re-used a repository that I had been using recently.
Thanks for spending time looking at these changes.
More information about the nio-dev