On 6/1/12 5:26 AM, Remy Maucherat wrote:
> On Fri, 2012-06-01 at 14:02 +0200, Greg Wilkins wrote:
>> On 1 June 2012 02:49, Rajiv Mordani<rajiv.mordani_at_oracle.com> wrote:
>>> We have discussed this in the past and what we said was b above.
>> It was discussed, but I must have missed the decision to go for b, nor
>> is it clear in the document or javadoc.
>>
>> I think this is poor choice as it means that a container and
>> application cannot control their memory footprint. Consider an
>> application that allocates some large byte arrays in some kind of
>> content cache. It can be configured with a maximum memory usage and
>> can be constrained. But when it starts sending these buffers
>> asynchronously, it does not know if a write will copy the data. It
>> might right the same content to an unlimited number of connections, so
>> it may do an unlimited number of copies. Such an application would
>> always be vulnerable to OOM and there is nothing the container or
>> application can do to prevent it other than limit the number of
>> connections - but the whole point of async IO is to increase
>> scalability not to limit connections!
> We get the idea, I believe. The idea is to limit the amount of changes,
> and provide something easy to use.
>
> The application should be concerned about how it sends its
> data. If the programmer cannot code, he will not be able to use this new
> async API. Since there's the servlet buffer, using a trivial loop over
> write(byte[], int off, int len) will not change anything to the actual
> IO, and your edge case is covered. This edge case is best covered with a
> sendfile-like API, to go along with the option b) which we much prefer
> [since it works better everywhere else]. I am ok with adding it, but
> others are apparently not.
I am open to discussing the sendFile later - but first let's get an EDR
out. We are way overdue.
>
>>> I have clarified this earlier as well and I think it is in the spec too -
>>> but the onWritePossible is called once for every write possible. Once
>>> invoked, it will not be invoked till a write operation is performed or the
>>> canWrite is flipped.
>>
>> The document says:
>>
>> void onWritePossible(). This method will be invoked by the container for
>> the associated NonBlockingOutputSink object when it is possible to write data.
>> This method will be invoked once for every write operation possible. Once a
>> write operation is performed, this method will subsequently be invoked by the
>> container when further writes are possible.
>>
>> The javadoc says:
>> void onWritePossible(ServletResponse response)
>> This callback will be invoked when data can be written without blocking.
>>
>> Neither those descriptions nor the one you gave in your response make
>> it very clear exactly what is meant. You say "will not be invoked
>> till a write operation is performed" and the document says "invoked
>> once for every write operation possible"
>> So all of these imply that onWritePossible will be called after every
>> write operations - is this correct? I thought the NIO.2 style was
>> rejected because of the expense of callbacks, yet this proposal will
>> have a callback per write?
>>
>> I had thought that the intention was that it would only be called if a
>> write had causes canWrite status to flip from true to false.
>> However, thinking about this now, that represents a horrible race
>> condition as canWrite might flip to false during the call to write,
>> but then might flip back to true before it is called, so the caller
>> will not know if onWritePossilble will be called or not. If the
>> intention is to have the callback happen only if the status is
>> flipped, then to avoid the race then I think we need to say that
>> onWritePossible is called ONLY if a call to canWrite() has returned
>> false. ie if the internal canWrite status momentarily is false,
>> that does not mean onWritePossible will be called, because if the
>> write completes before canWrite() is called, then there is no need for
>> the callback.
> Yes, this is why I said that the callback must happen after the
> canWrite() method is called and returns false, this is actually the
> trigger [it is not when the flag flips]. This is an important trick to
> make it work.
>
> I think I described the algorithm in detail already.
Yes. Thanks Remy.
>
>> Also the document and javadoc has no text explaining the context of
>> the callback thread. I assume that it is a thread fully anointed
>> with servletcontext scope an security stuff? But for a suspended
>> request that may have passed through several contexts, which context
>> will this be? Also if the model really is a callback after every
>> write, then many such IO frameworks will use the thread that called
>> write to do the callback if the write completed. If we allow this,
>> then we may have to do similar stuff to NIO.2 to prevent unconstrained
>> stack growth if the callback does another write that does another
>> callback etc. This problem is avoided if onWritePossible is only
>> called if canWrite returns false.
> Yes, all callbacks are in the EE context, like all Servlet invocations.
> We already discussed this and the implications of the callback count,
> and the need to limit it, please look at the archives. I think you were
> actually the one advocating a NIO2 style API, with a callback after
> every write.
>
>> Is there a reference implementation of this available anywhere?
>> What about some sample code that uses this API?
> You can look at JBoss AS 7, it is not the same API, the behavior is
> different sometimes, but it has a similar concept where the usual
> blocking IO is extend with async capabilities [and protocol upgrade
> support].
>