6916202: (so) improve scatter/gather implementation
Alan.Bateman at oracle.com
Tue Jul 27 08:35:38 PDT 2010
Chris Hegarty wrote:
> To clarify ( sent in a private mail ), the bug number is 6971825 and
> the webrev is:
Thanks for looking at this one. Sorry about the bugID, I did indeed get
that wrong in the mail.
> Util.getTemporaryDirectBuffer. If reading/writing in non
> scattering/gathering it looks like a new buffer will be created every
> time. Should L64 be i <= count?
The buffers are 0..count-1 so I think this is okay.
> Indentation for "throw new IndexOutOfBoundsException();" in
> DatagramChannelImpl and FileChannelImpl seems to be only three spaces.
I'll fix that (I just moved the check and it looks like the indentation
has always been wrong).
> The temporary buffer cache is no longer SoftRefs. Do you see any
> potential side effect of this? Is this deliberate?
A soft reference works well for caches in the java heap but it hasn't
worked well for direct buffers (that are allocated outside of the java
> IOUtil.write/read. Releasing the shadow buffer and clearing the vec
> refs could just be done in the finally block with need for completed.
> But then your committing to a second loop, where as with what you have
> the second loop is only necessary if there is a failure, right?
It could be done this way except that there is potential for a runtime
exception when notifying the buffers - for example buggy code that
modifies the position or limit while an I/O operation is done on the
buffer (it has a happened a few times, and very difficult to diagnose).
> It looks like now you can only read/write with a limit of 8 buffers,
> where as before it just spilled over to creating short lived direct
> buffers. I think this should be sufficient, just an observation that
> we don't spill anymore.
Yes, once warmed up then we shouldn't seen any malloc/free going on,
even for applications using a huge number of buffers. Most applications
using this API don't use scatter/gather and the main difference for
these cases is that we're caching at most 1 buffer per thread whereas it
was caching up to 3 buffers per thread, which didn't make sense.
More information about the nio-dev