You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by Sam Berlin <sb...@gmail.com> on 2008/02/12 19:49:22 UTC
NHttpXXHandler Impl Improvements?
Hi Folks,
I think this issue might have been brought up awhile ago (by Steffen,
maybe?), but as we're coming around to using HttpCore for downloading,
I think it's worthwhile to bring it up again.
It seems that there are two default implemenations for
NHttpServiceHandler or NHttpClientHandler, a Buffering & Throttling
variety. The throttling variety requires a thread for each request
and the buffering variety buffers the entire contents in memory.
We've modified the BufferingHttpServiceHandler to provide asynchronous
handling by using a new interface that extends HttpEntity with the
additional methods:
int consumeContent(ContentDecoder decoder, IOControl ioctrl);
void produceContent(ContentEncoder encoder, IOControl ioctrl);
void finished();
In sendResponse where it normally would ask the entity to write itself
to an outputStream, it instead checks to see if the entity is an
instance of this new HttpEntity subinterface, and if so, sets it
within the ConnState. In the outputReady method, it retrieves the
entity from the ConnState and asks the entity to produce content
itself (via the above produceContent method), instead of asking the
ContentOutputBuffer to do it. This bypasses the buffer and allows the
entity some finer-grained control over the writing. The control is
better because of access to the IOControl object. This is
particularly useful when reading from disk -- if multiple files are
being uploaded at once and there's a disk-reading service that is
backed up, the entity can ask the control to suspend output requests
until the disk service has returned with some data that can be
written. As a neat optimization, bypassing the ContentOutputBuffer
avoids allocating some extraneous ByteBuffers, since the entity can
write directly into the ContentEncoder.
What are the chances that changes like this can go into HttpNIO
itself? Or do you see any way something like this can be done within
the existing framework of the ContentOutputBuffer? (I'm not married
to the optimization of avoiding allocating the ByteBuffers, but do
require access to the IOControl to suspend/resume input & output.)
My reasons for asking are mainly because right now we maintain our own
implementation of NHttpServiceHandler (and shortly,
NHttpClientHandler), but I'd prefer to see them folded into the
distro, rather than keeping our fork updated.
Thanks,
Sam
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: NHttpXXHandler Impl Improvements?
Posted by Sam Berlin <sb...@gmail.com>.
Ok, I'm going to tentatively use:
NHttpEntity extends HttpEntity { void finish(); }
ContentConsumingNHttpEntity extends NHttpEntity { consumeContent(..); }
ContentProducingNHttpEntity extends NHttpEntity { produceContent(..); }
The names are a bit iffy, but can't think of anything better at the
moment. Separating the interfaces is a good idea -- should make the
end result a lot cleaner.
Sam
On 2/13/08, Oleg Kalnichevski <ol...@apache.org> wrote:
> Sam Berlin wrote:
> > After looking through things, there seems to already be somewhat of an
> > infrastructure for NIO entities using the
> > o.a.http.nio.entity.HttpNIOEntity and its subclasses. Those entities
> > appear to expose a 'getChannel' method (used right now just in the
> > NHttpFileServer example). I propose a change in that interface, to
> > make it a little more analagous to the HttpEntity interface.
> > Currently HttpEntity has a blocking 'writeTo' method and a
> > 'getContent' method returning the InputStream. HttpNIOEntity just has
> > the 'getChannel' method, but no non-blocking 'produceContent' method.
> >
> > Is it OK right now to completely change interfaces/classes in
> > http-nio?
>
> Not unless we absolutely have to. Preferably we should initially
> deprecate those interfaces we intent to delete in the future.
>
>
> If so, I would like to make HttpNIOEntity into a copy of
> > the HttpNIOEntity we're using internally. That is:
> >
>
> I kind of dislike that this interface has both produceContent and
> consumeContent methods. Would it be ok if we split things into two
> distinct interfaces?
>
>
> How about this?
>
> interface ContentProducingHttpNIOEntity extends HttpNIOEntity {
>
> void produceContent(ContentEncoder encoder, IOControl ioctrl)
> throws IOException;
>
> void finish();
>
> }
>
>
> interface ContentConsumingHttpNIOEntity extends HttpNIOEntity {
>
> int consumeContent(ContentDecoder decoder, IOControl ioctrl)
> throws IOException;
>
> void finish();
>
> }
>
> Better names are very welcome.
>
> Oleg
>
> > /**
> > * Defines an interface for event based HTTP entities.
> > */
> > public interface HttpNIOEntity extends HttpEntity {
> >
> > /**
> > * Reads data from <code>decoder</code>.
> > *
> > * @param decoder the decoder for reading input
> > * @param ioctrl I/O control for suspending and resuming events
> > * @return the number of bytes read
> > * @throws IOException indicates an I/O error which will abort the
> > * connection
> > */
> > int consumeContent(ContentDecoder decoder, IOControl ioctrl)
> > throws IOException;
> >
> > /**
> > * Writes data to <code>encoder</code>.
> > *
> > * @param encoder the encoder for writing output
> > * @param ioctrl I/O control for suspending and resuming events
> > * @throws IOException indicates an I/O error which will abort the
> > * connection
> > */
> > void produceContent(ContentEncoder encoder, IOControl ioctrl)
> > throws IOException;
> >
> > /**
> > * Invoked when processing has completed. This completes the transfer of the
> > * entity and is invoked when processing finishes normally, i.e.
> > * {@link ContentEncoder#isCompleted()} or
> > * {@link ContentDecoder#isCompleted()} return true, or when an exception
> > * occurs.
> > */
> > void finished();
> >
> > }
> >
> > This moves the burden of reading/writing non-blocking content to the
> > entity itself (much like it already does for writing, using writeTo).
> > Retrieving the channel is removed because it becomes unnecessary (and
> > could be cumbersome for some entities). The 'finished' method is to
> > signal that the entity can reclaim resources, and would be called when
> > the connection closes or the entity has completed its writing|reading
> > (according to the encoder|decoder).
> >
> > Given that all entities are created within the Service|ClientHandlers
> > or within NHttpConnectionBase, it should be pretty easy to require
> > that all entities used in an NIO context implement this new
> > HttpNIOEntity. And the fact that the interface & some implementations
> > already exist makes things a little easier, because the planning
> > already is done. It's just the use of the classes (and when they're
> > used) that needs to change.
> >
> > Any thoughts on the approach? I haven't fully examined the 'client'
> > side of it yet -- am mostly approaching it on the server-side right
> > now, so there could be hitches there.
> >
> > Sam
> >
> > On 2/12/08, Sam Berlin <sb...@gmail.com> wrote:
> >>> Pretty much 100% if this code comes with a reasonable test coverage.
> >>> Moreover, the contributor of this code will qualify for a full
> >>> committership on the project for such such a substantial contribution,
> >>> in my opinion.
> >> Cool, Ok. I'll see how it goes (and if the buffered scheme can be deprecated).
> >>
> >> Sam
> >>
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
> > For additional commands, e-mail: dev-help@hc.apache.org
> >
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
> For additional commands, e-mail: dev-help@hc.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: NHttpXXHandler Impl Improvements?
Posted by Oleg Kalnichevski <ol...@apache.org>.
Sam Berlin wrote:
> After looking through things, there seems to already be somewhat of an
> infrastructure for NIO entities using the
> o.a.http.nio.entity.HttpNIOEntity and its subclasses. Those entities
> appear to expose a 'getChannel' method (used right now just in the
> NHttpFileServer example). I propose a change in that interface, to
> make it a little more analagous to the HttpEntity interface.
> Currently HttpEntity has a blocking 'writeTo' method and a
> 'getContent' method returning the InputStream. HttpNIOEntity just has
> the 'getChannel' method, but no non-blocking 'produceContent' method.
>
> Is it OK right now to completely change interfaces/classes in
> http-nio?
Not unless we absolutely have to. Preferably we should initially
deprecate those interfaces we intent to delete in the future.
If so, I would like to make HttpNIOEntity into a copy of
> the HttpNIOEntity we're using internally. That is:
>
I kind of dislike that this interface has both produceContent and
consumeContent methods. Would it be ok if we split things into two
distinct interfaces?
How about this?
interface ContentProducingHttpNIOEntity extends HttpNIOEntity {
void produceContent(ContentEncoder encoder, IOControl ioctrl)
throws IOException;
void finish();
}
interface ContentConsumingHttpNIOEntity extends HttpNIOEntity {
int consumeContent(ContentDecoder decoder, IOControl ioctrl)
throws IOException;
void finish();
}
Better names are very welcome.
Oleg
> /**
> * Defines an interface for event based HTTP entities.
> */
> public interface HttpNIOEntity extends HttpEntity {
>
> /**
> * Reads data from <code>decoder</code>.
> *
> * @param decoder the decoder for reading input
> * @param ioctrl I/O control for suspending and resuming events
> * @return the number of bytes read
> * @throws IOException indicates an I/O error which will abort the
> * connection
> */
> int consumeContent(ContentDecoder decoder, IOControl ioctrl)
> throws IOException;
>
> /**
> * Writes data to <code>encoder</code>.
> *
> * @param encoder the encoder for writing output
> * @param ioctrl I/O control for suspending and resuming events
> * @throws IOException indicates an I/O error which will abort the
> * connection
> */
> void produceContent(ContentEncoder encoder, IOControl ioctrl)
> throws IOException;
>
> /**
> * Invoked when processing has completed. This completes the transfer of the
> * entity and is invoked when processing finishes normally, i.e.
> * {@link ContentEncoder#isCompleted()} or
> * {@link ContentDecoder#isCompleted()} return true, or when an exception
> * occurs.
> */
> void finished();
>
> }
>
> This moves the burden of reading/writing non-blocking content to the
> entity itself (much like it already does for writing, using writeTo).
> Retrieving the channel is removed because it becomes unnecessary (and
> could be cumbersome for some entities). The 'finished' method is to
> signal that the entity can reclaim resources, and would be called when
> the connection closes or the entity has completed its writing|reading
> (according to the encoder|decoder).
>
> Given that all entities are created within the Service|ClientHandlers
> or within NHttpConnectionBase, it should be pretty easy to require
> that all entities used in an NIO context implement this new
> HttpNIOEntity. And the fact that the interface & some implementations
> already exist makes things a little easier, because the planning
> already is done. It's just the use of the classes (and when they're
> used) that needs to change.
>
> Any thoughts on the approach? I haven't fully examined the 'client'
> side of it yet -- am mostly approaching it on the server-side right
> now, so there could be hitches there.
>
> Sam
>
> On 2/12/08, Sam Berlin <sb...@gmail.com> wrote:
>>> Pretty much 100% if this code comes with a reasonable test coverage.
>>> Moreover, the contributor of this code will qualify for a full
>>> committership on the project for such such a substantial contribution,
>>> in my opinion.
>> Cool, Ok. I'll see how it goes (and if the buffered scheme can be deprecated).
>>
>> Sam
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
> For additional commands, e-mail: dev-help@hc.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: NHttpXXHandler Impl Improvements?
Posted by Sam Berlin <sb...@gmail.com>.
After looking through things, there seems to already be somewhat of an
infrastructure for NIO entities using the
o.a.http.nio.entity.HttpNIOEntity and its subclasses. Those entities
appear to expose a 'getChannel' method (used right now just in the
NHttpFileServer example). I propose a change in that interface, to
make it a little more analagous to the HttpEntity interface.
Currently HttpEntity has a blocking 'writeTo' method and a
'getContent' method returning the InputStream. HttpNIOEntity just has
the 'getChannel' method, but no non-blocking 'produceContent' method.
Is it OK right now to completely change interfaces/classes in
http-nio? If so, I would like to make HttpNIOEntity into a copy of
the HttpNIOEntity we're using internally. That is:
/**
* Defines an interface for event based HTTP entities.
*/
public interface HttpNIOEntity extends HttpEntity {
/**
* Reads data from <code>decoder</code>.
*
* @param decoder the decoder for reading input
* @param ioctrl I/O control for suspending and resuming events
* @return the number of bytes read
* @throws IOException indicates an I/O error which will abort the
* connection
*/
int consumeContent(ContentDecoder decoder, IOControl ioctrl)
throws IOException;
/**
* Writes data to <code>encoder</code>.
*
* @param encoder the encoder for writing output
* @param ioctrl I/O control for suspending and resuming events
* @throws IOException indicates an I/O error which will abort the
* connection
*/
void produceContent(ContentEncoder encoder, IOControl ioctrl)
throws IOException;
/**
* Invoked when processing has completed. This completes the transfer of the
* entity and is invoked when processing finishes normally, i.e.
* {@link ContentEncoder#isCompleted()} or
* {@link ContentDecoder#isCompleted()} return true, or when an exception
* occurs.
*/
void finished();
}
This moves the burden of reading/writing non-blocking content to the
entity itself (much like it already does for writing, using writeTo).
Retrieving the channel is removed because it becomes unnecessary (and
could be cumbersome for some entities). The 'finished' method is to
signal that the entity can reclaim resources, and would be called when
the connection closes or the entity has completed its writing|reading
(according to the encoder|decoder).
Given that all entities are created within the Service|ClientHandlers
or within NHttpConnectionBase, it should be pretty easy to require
that all entities used in an NIO context implement this new
HttpNIOEntity. And the fact that the interface & some implementations
already exist makes things a little easier, because the planning
already is done. It's just the use of the classes (and when they're
used) that needs to change.
Any thoughts on the approach? I haven't fully examined the 'client'
side of it yet -- am mostly approaching it on the server-side right
now, so there could be hitches there.
Sam
On 2/12/08, Sam Berlin <sb...@gmail.com> wrote:
> > Pretty much 100% if this code comes with a reasonable test coverage.
> > Moreover, the contributor of this code will qualify for a full
> > committership on the project for such such a substantial contribution,
> > in my opinion.
>
> Cool, Ok. I'll see how it goes (and if the buffered scheme can be deprecated).
>
> Sam
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: NHttpXXHandler Impl Improvements?
Posted by Sam Berlin <sb...@gmail.com>.
> Pretty much 100% if this code comes with a reasonable test coverage.
> Moreover, the contributor of this code will qualify for a full
> committership on the project for such such a substantial contribution,
> in my opinion.
Cool, Ok. I'll see how it goes (and if the buffered scheme can be deprecated).
Sam
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: NHttpXXHandler Impl Improvements?
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Tue, 2008-02-12 at 13:49 -0500, Sam Berlin wrote:
> Hi Folks,
>
> I think this issue might have been brought up awhile ago (by Steffen,
> maybe?), but as we're coming around to using HttpCore for downloading,
> I think it's worthwhile to bring it up again.
>
> It seems that there are two default implemenations for
> NHttpServiceHandler or NHttpClientHandler, a Buffering & Throttling
> variety. The throttling variety requires a thread for each request
> and the buffering variety buffers the entire contents in memory.
> We've modified the BufferingHttpServiceHandler to provide asynchronous
> handling by using a new interface that extends HttpEntity with the
> additional methods:
>
> int consumeContent(ContentDecoder decoder, IOControl ioctrl);
> void produceContent(ContentEncoder encoder, IOControl ioctrl);
> void finished();
>
> In sendResponse where it normally would ask the entity to write itself
> to an outputStream, it instead checks to see if the entity is an
> instance of this new HttpEntity subinterface, and if so, sets it
> within the ConnState. In the outputReady method, it retrieves the
> entity from the ConnState and asks the entity to produce content
> itself (via the above produceContent method), instead of asking the
> ContentOutputBuffer to do it. This bypasses the buffer and allows the
> entity some finer-grained control over the writing. The control is
> better because of access to the IOControl object. This is
> particularly useful when reading from disk -- if multiple files are
> being uploaded at once and there's a disk-reading service that is
> backed up, the entity can ask the control to suspend output requests
> until the disk service has returned with some data that can be
> written. As a neat optimization, bypassing the ContentOutputBuffer
> avoids allocating some extraneous ByteBuffers, since the entity can
> write directly into the ContentEncoder.
>
> What are the chances that changes like this can go into HttpNIO
> itself?
Pretty much 100% if this code comes with a reasonable test coverage.
Moreover, the contributor of this code will qualify for a full
committership on the project for such such a substantial contribution,
in my opinion.
> Or do you see any way something like this can be done within
> the existing framework of the ContentOutputBuffer? (I'm not married
> to the optimization of avoiding allocating the ByteBuffers, but do
> require access to the IOControl to suspend/resume input & output.)
>
I'll happily deprecate the buffering kind of protocol handlers in favor
of the new one, instead.
> My reasons for asking are mainly because right now we maintain our own
> implementation of NHttpServiceHandler (and shortly,
> NHttpClientHandler), but I'd prefer to see them folded into the
> distro, rather than keeping our fork updated.
>
> Thanks,
> Sam
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
> For additional commands, e-mail: dev-help@hc.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org