Improving perf of FileChannel.transferTo() on Windows
Alan.Bateman at oracle.com
Sun Nov 23 10:15:38 UTC 2014
On 14/11/2014 16:51, Kirk Shoop (MS OPEN TECH) wrote:
> *From:*Alan Bateman [mailto:Alan.Bateman at oracle.com]
> I think this looks much better.
> One thing that needs a closer look is the loop around TransmitFile. If
> it fails on the second or subsequent call then transferTo will fail
> after already sending transmitting bytes. For transferTo then it's
> okay to complete without having transmitted all bytes and maybe it
> would be better to just cap it at 2GB. Any robust code using
> transferTo should check the return value. (BTW: You can use
> java_lang_Integer_MAX_VALUE to avoid the repeating the constant).
> Good point, Valery removed the loop.
> In transferToDirectly then I don't think you need to check if the
> target is a SelectableChannel if you move call to
> nd.canTransferToDirectly to just above where it handles SelChImpl.
> One suggestion is to top transferToDirectlyEnabled and instead jave
> canTransferToDirectly return true then the property is enabled and the
> target change is SelectableChannel configured blocking.
> A minor comment is that transferToDirectlyChangesChannelPosition is a
> bit inconsistent with methods like needPositionLock so you might be
> able to come up with something a bit closer to that,
> transferDirectNeedsPositionLock maybe?
> Valery chose 'transferToDirectlyNeedsPositionRestoring', but this can
> be changed.
> While looking at this, I wondered, could the position lock and
> position manipulation in Java be skipped altogether if
> 'Java_sun_nio_ch_FileChannelImpl_transferTo0' used
> 'Java_sun_nio_ch_FileChannelImpl_position0' to restore the position to
> the original point?
> A few style/formatting issues that we can sort out later, otherwise
> the approach looks okay to me.
> Happy to take those changes anytime.
Sorry for the delay getting back to you on this, I've been busy with
I think the updated version looks good and you've addressed the issues
that I was concerned about. One other potential issue that came to mind
later is that the blocking mode of the SocketChannel could change to
non-blocking at or around the time that transferTo is called. It would
be possible to restructure this to hold the blockingLock but I think it
makes the locking more complicated. Worst case with this "bug" is that
transferTo blocks at around the time the blocking mode is changed to
non-blocking. There is a bigger (and longstanding) issue in this area
related to async close of the target channel during a transfer and
if/when that gets looked at then I think it is the time to address the
blocking lock issue too.
I had to re-base your patch to get it working with the current jdk9/dev
tip, that is only because of other recent changes in this area. I
tweaked some minor formatting issues and used the opportunity to replace
the imports that use wildcards. I don't have a strong opinion on the
method to determine if the position changes except that
transferToDirectlyNeedsPositionLock is a bit more consistent with the
current naming. Here is the updated patch (only minor edits as I said):
I've run this patch through our build+test system to make sure that it
works on the 7+ platforms that we normally care about and I don't see
any issues. I've also run the jdk_nio tests with the property enabled so
that all tests using transferTo use TransmitFile when the channel is
Let me know if you are okay with this?
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the nio-dev