httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject Re: PLEASE READ: Filter I/O
Date Thu, 22 Jun 2000 21:38:09 GMT
On Thu, Jun 22, 2000 at 07:48:06AM -0700, rbb@covalent.net wrote:
> 
> > Sorry, but I have to give a -1 again.
> > 
> > I realize how this is really holding up the filtering concept, so I feel a
> > big reponsibility to provide real, working code for the alternative scheme.
> > I'm putting the mod_dav integration on hold until I code up a patch for the
> > link-based filtering. I feel it is a bit unfair to veto without coughing up
> > some code, so that'll be first priority for me.
> 
> I can not argue against a non existant patch.  I am not discussing this
> anymore until I have seen a patch that implements the link based scheme.

There are issues with the hook-based scheme (detailed in my emails). Those
can be discussed and corrected in your patch, independent of a link-based
patch.

> Although I can't make any well reasoned arguments against this patch, I
> can without a doubt make one against the design.  The recursive nature of
> your design will slow this server to a crawl on some platforms.  Take a
> look at the Sparc architecture.  This architecture uses register windows,
> and these fall down when making recursive calls.

I really don't know how to respond to this. This is not reality. Can you
corroborate this position with any evidence?

Entering a function multiple times on the same thread of execution is simply
NOT a problem.

> > Also, as I stated last week (before leaving for a few days of vacation), I'm
> > going to continue with some of the "common" items to simplify comparison and
> > review. I'll repost those items because I've got some questions/concerns for
> > people to comment on.
> 
> I have already veto'ed all of those patches.

Sorry, but I took no action on that because I was away, and it seemed from
Ken's email that you needed to say "why" a bit more.

> If you want a technical
> reason, here it is.  As I have already proven with my last patch, these
> changes are completely bogus unless we use the link based scheme.

Those changes are still needed. Your patch does not prove/demonstrate
anything -- you need to include some calls to check_first_conn_error() for
your patch to be *correct*.

In the first posting of your recent patch, you changed ap_rvputs() over to
ap_bvputs(). At that point, I stated that was an improper change because the
two semantics do not match. check_first_conn_error() is a necessary step.
You need it in filter_io() and you need it for other calls to ap_b*().

Let's take a concrete example of the ap_rvputs() change to
ap_send_header_field. Please take a look at the HEAD version of ap_rvputs().
The "aborted" check, the call to ap_vbputstrs(), the error check and call to
check_first_conn_error(), and the SET_BYTES_SENT() all need to be done in
ap_send_header_field(). In both filter schemes.

I was working on adding a function, call it checked_bvputstrs() that would
do this work. checked_bvputstrs() is just a copy of ap_rvputs(). Either of
the filter patches will then modify ap_rvputs() to do its magic. BUT!
checked_bvputstrs() remains untouched and ap_send_header_field() can
continue to operate properly.

> The
> function that you created in your last patch to do this stuff is
> completely unnecessary and adds a wasted function call when using the hook
> based scheme.

See above.

Also, it only adds the call when an error occurs (i.e. it does not get
called during normal execution, so it should not impact performance). At
that point, it aborts the connection and "if aborted" tests at the beginning
of ap_r*() will prevent continued work in those functions.

At this point, I am at a quandary. I can roll back the
check_first_conn_error() thing, although with some difficulty because of
later commits. I'm willing to do so, but it seems that I've addressed your
technical justifications for the veto. It would be helpful if you could do
one of two things: rescind the veto (easiest for all involved), or you could
explain any further issues (i.e. whether your original technical issue still
exists, or you have further issues).

> These "common" things that you want to change have VERY
> different solutions based on which scheme is chosen finally.

I understand and have been choosing changes that both approaches will need.

> The patch I
> submitted yesterday rips your last "common" change out completely.  At the
> very least do not commit until the changes have been posted first.  This
> way I can give valid technical reasons for why those common changes aren't
> common at all.

Sigh. This is a basic lack of trust, which is normally implied by the
commit-then-review model. I will do RTC, but only under protest.

I have said, have done, and plan to make no changes that are not applicable
to both schemes. I am doing this for several reasons:

1) reduce the "final" filtering patch set, thus aiding its reviewability
2) producing some forward motion on those items which *can* move forward
3) by pulling out these changes from the "full" filtering patch, they can be
   individually reviewed for correctness

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Mime
View raw message