You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by rb...@covalent.net on 2001/02/21 14:18:58 UTC
Re: cvs commit: httpd-2.0/modules/http http_protocol.c
On 21 Feb 2001 stoddard@apache.org wrote:
> stoddard 01/02/21 13:14:33
>
> Modified: modules/http http_protocol.c
> Log:
> BUFF is gone!
BUFF may be gone, but were these issues ever REALLY investigated?
Ryan
>
> Revision Changes Path
> 1.299 +2 -23 httpd-2.0/modules/http/http_protocol.c
>
> Index: http_protocol.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/modules/http/http_protocol.c,v
> retrieving revision 1.298
> retrieving revision 1.299
> diff -u -r1.298 -r1.299
> --- http_protocol.c 2001/02/16 04:26:39 1.298
> +++ http_protocol.c 2001/02/21 21:14:33 1.299
> @@ -1223,23 +1223,9 @@
> * read(). B_SAFEREAD ensures that the BUFF layer flushes if it will
> * have to block during a read.
> */
> -#if 0
> - /* XXX: I am 99% sure that these are already taken care of, but I want to
> - * really investigate them still. Removing them from the code doesn't
> - * hurt however, because nothing is using BUFF anymore.
> - */
> - ap_bsetflag(conn->client, B_SAFEREAD, 1);
> - ap_bflush(conn->client);
> -#endif
> +
> while ((len = ap_getline(l, sizeof(l), r, 0)) <= 0) {
> if (len < 0) { /* includes EOF */
> -#if 0
> - /* XXX: I am 99% sure that these are already taken care of, but I want to
> - * really investigate them still. Removing them from the code doesn't
> - * hurt however, because nothing is using BUFF anymore.
> - */
> - ap_bsetflag(conn->client, B_SAFEREAD, 0);
> -#endif
> /* this is a hack to make sure that request time is set,
> * it's not perfect, but it's better than nothing
> */
> @@ -1260,17 +1246,10 @@
> #endif
> */
>
> -#if 0
> - /* XXX: I am 99% sure that these are already taken care of, but I want to
> - * really investigate them still. Removing them from the code doesn't
> - * hurt however, because nothing is using BUFF anymore.
> - */
> - ap_bsetflag(conn->client, B_SAFEREAD, 0);
> -#endif
> -
> r->request_time = apr_time_now();
> r->the_request = apr_pstrdup(r->pool, l);
> r->method = ap_getword_white(r->pool, &ll);
> +
> #if 0
> /* XXX If we want to keep track of the Method, the protocol module should do
> * it. That support isn't in the scoreboard yet. Hopefully next week
>
>
>
>
>
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: cvs commit: httpd-2.0/modules/http http_protocol.c
Posted by rb...@covalent.net.
On Thu, 22 Feb 2001 rbb@covalent.net wrote:
> On Thu, 22 Feb 2001, Bill Stoddard wrote:
>
> > > > > stoddard 01/02/21 13:14:33
> > > > >
> > > > > Modified: modules/http http_protocol.c
> > > > > Log:
> > > > > BUFF is gone!
> > > >
> > > > BUFF may be gone, but were these issues ever REALLY investigated?
> > > >
> > > > Ryan
> > > >
> >
> > SAFEREAD is an optimiation for handling pipelined requests. When we receive a
> > pipelined request, we want to pipeline the respose back (e.g., combine
> > mulltiple responses in a single buffer until thebuffer is filled before
> > flushing to the network).
> >
> > Here is how SAFEREAD mode worked
> >
> > Issuing a bread() on a BUFF in SAFEREAD mode caused the BUFF code to first
> > issue a select() on the socket to see if there was more bytes available (i.e.,
> > a pipelined request). If more bytes were available, bread() returned those
> > bytes. If no bytes were available, any response hanging around in BUFF was
> > flushed before the blocking read was issued. Fairly straightforward.
> >
> > check_pipeline_flush() in http_request.c is intended to do the same thing, but
> > I've not actually tested it because I don't like the function is implemented.
> > check_pipeline_flush() issues an ap_get_brigade(r->input_filters, bb,
> > AP_MODE_PEEK) which seems overkill. This function would be much cleaner if
> > it used an apr_select call on the socket to see if bytes were available to be
> > read. If no bytes were available, then send the flush down the filter chain.
> > If bytes are available, handle the crlf on the end of POST requests in the
> > input chain rather than in the AP_MODE_PEEK code. In fact, I would suggest
> > getting rid of AP_MODE_PEEK altogether.
>
> Doing a select won't do what you want it to do. There is a chance that we
> have already read the second request from the socket, and it is sitting in
> a lower filter. A higher filter can NOT just skip the rest of the filters
> and go straight to the network, it must go through the filter chain.
BTW, the pipeline_flush stuff does work, I have tested it quite a bit
already. Not that more testing isn't always appreciated.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: cvs commit: httpd-2.0/modules/http http_protocol.c
Posted by rb...@covalent.net.
On Thu, 22 Feb 2001, Bill Stoddard wrote:
> > > > stoddard 01/02/21 13:14:33
> > > >
> > > > Modified: modules/http http_protocol.c
> > > > Log:
> > > > BUFF is gone!
> > >
> > > BUFF may be gone, but were these issues ever REALLY investigated?
> > >
> > > Ryan
> > >
>
> SAFEREAD is an optimiation for handling pipelined requests. When we receive a
> pipelined request, we want to pipeline the respose back (e.g., combine
> mulltiple responses in a single buffer until thebuffer is filled before
> flushing to the network).
>
> Here is how SAFEREAD mode worked
>
> Issuing a bread() on a BUFF in SAFEREAD mode caused the BUFF code to first
> issue a select() on the socket to see if there was more bytes available (i.e.,
> a pipelined request). If more bytes were available, bread() returned those
> bytes. If no bytes were available, any response hanging around in BUFF was
> flushed before the blocking read was issued. Fairly straightforward.
>
> check_pipeline_flush() in http_request.c is intended to do the same thing, but
> I've not actually tested it because I don't like the function is implemented.
> check_pipeline_flush() issues an ap_get_brigade(r->input_filters, bb,
> AP_MODE_PEEK) which seems overkill. This function would be much cleaner if
> it used an apr_select call on the socket to see if bytes were available to be
> read. If no bytes were available, then send the flush down the filter chain.
> If bytes are available, handle the crlf on the end of POST requests in the
> input chain rather than in the AP_MODE_PEEK code. In fact, I would suggest
> getting rid of AP_MODE_PEEK altogether.
Doing a select won't do what you want it to do. There is a chance that we
have already read the second request from the socket, and it is sitting in
a lower filter. A higher filter can NOT just skip the rest of the filters
and go straight to the network, it must go through the filter chain.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: cvs commit: httpd-2.0/modules/http http_protocol.c
Posted by Bill Stoddard <bi...@wstoddard.com>.
> > > stoddard 01/02/21 13:14:33
> > >
> > > Modified: modules/http http_protocol.c
> > > Log:
> > > BUFF is gone!
> >
> > BUFF may be gone, but were these issues ever REALLY investigated?
> >
> > Ryan
> >
SAFEREAD is an optimiation for handling pipelined requests. When we receive a
pipelined request, we want to pipeline the respose back (e.g., combine
mulltiple responses in a single buffer until thebuffer is filled before
flushing to the network).
Here is how SAFEREAD mode worked
Issuing a bread() on a BUFF in SAFEREAD mode caused the BUFF code to first
issue a select() on the socket to see if there was more bytes available (i.e.,
a pipelined request). If more bytes were available, bread() returned those
bytes. If no bytes were available, any response hanging around in BUFF was
flushed before the blocking read was issued. Fairly straightforward.
check_pipeline_flush() in http_request.c is intended to do the same thing, but
I've not actually tested it because I don't like the function is implemented.
check_pipeline_flush() issues an ap_get_brigade(r->input_filters, bb,
AP_MODE_PEEK) which seems overkill. This function would be much cleaner if
it used an apr_select call on the socket to see if bytes were available to be
read. If no bytes were available, then send the flush down the filter chain.
If bytes are available, handle the crlf on the end of POST requests in the
input chain rather than in the AP_MODE_PEEK code. In fact, I would suggest
getting rid of AP_MODE_PEEK altogether.
Bill
Re: cvs commit: httpd-2.0/modules/http http_protocol.c
Posted by Bill Stoddard <bi...@wstoddard.com>.
Assumed so, but I'll give a look to make sure.
Bill
> On 21 Feb 2001 stoddard@apache.org wrote:
>
> > stoddard 01/02/21 13:14:33
> >
> > Modified: modules/http http_protocol.c
> > Log:
> > BUFF is gone!
>
> BUFF may be gone, but were these issues ever REALLY investigated?
>
> Ryan
>
> >
> > Revision Changes Path
> > 1.299 +2 -23 httpd-2.0/modules/http/http_protocol.c
> >
> > Index: http_protocol.c
> > ===================================================================
> > RCS file: /home/cvs/httpd-2.0/modules/http/http_protocol.c,v
> > retrieving revision 1.298
> > retrieving revision 1.299
> > diff -u -r1.298 -r1.299
> > --- http_protocol.c 2001/02/16 04:26:39 1.298
> > +++ http_protocol.c 2001/02/21 21:14:33 1.299
> > @@ -1223,23 +1223,9 @@
> > * read(). B_SAFEREAD ensures that the BUFF layer flushes if it
will
> > * have to block during a read.
> > */
> > -#if 0
> > - /* XXX: I am 99% sure that these are already taken care of, but I
want to
> > - * really investigate them still. Removing them from the code
doesn't
> > - * hurt however, because nothing is using BUFF anymore.
> > - */
> > - ap_bsetflag(conn->client, B_SAFEREAD, 1);
> > - ap_bflush(conn->client);
> > -#endif
> > +
> > while ((len = ap_getline(l, sizeof(l), r, 0)) <= 0) {
> > if (len < 0) { /* includes EOF */
> > -#if 0
> > - /* XXX: I am 99% sure that these are already taken care of, but I
want to
> > - * really investigate them still. Removing them from the code
doesn't
> > - * hurt however, because nothing is using BUFF anymore.
> > - */
> > - ap_bsetflag(conn->client, B_SAFEREAD, 0);
> > -#endif
> > /* this is a hack to make sure that request time is set,
> > * it's not perfect, but it's better than nothing
> > */
> > @@ -1260,17 +1246,10 @@
> > #endif
> > */
> >
> > -#if 0
> > - /* XXX: I am 99% sure that these are already taken care of, but I
want to
> > - * really investigate them still. Removing them from the code
doesn't
> > - * hurt however, because nothing is using BUFF anymore.
> > - */
> > - ap_bsetflag(conn->client, B_SAFEREAD, 0);
> > -#endif
> > -
> > r->request_time = apr_time_now();
> > r->the_request = apr_pstrdup(r->pool, l);
> > r->method = ap_getword_white(r->pool, &ll);
> > +
> > #if 0
> > /* XXX If we want to keep track of the Method, the protocol module
should do
> > * it. That support isn't in the scoreboard yet. Hopefully next
week
> >
> >
> >
> >
> >
>
>
>
____________________________________________________________________________
___
> Ryan Bloom rbb@apache.org
> 406 29th St.
> San Francisco, CA 94131
> --------------------------------------------------------------------------
-----
>