httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject layered I/O (was: cvs commit: ...)
Date Sun, 26 Mar 2000 21:59:31 GMT
On Sun, 26 Mar 2000 rbb@apache.org wrote:
> > -1
> > 
> > Per my previous mail, this design imposes too much knowledge on the module
> > writer. Also, the handler changing thing before returning RERUN_HANDLERS
> > seems a bit hacky.
> 
> I don't understand why this design imposes too much knowledge on module
> writers, as per my response to your last mail.  There are 2 changes that
> must be made, (well three if you include returning RERUN_HANDLERS).

As Ralf said, what is going to happen is that ALL content-generating
modules will have to return that code. They will ALL have to monkey with
r->handler before returning. Since they will always return RERUN_HANDLERS,
then Apache is going to LOOP until it doesn't find other handlers.

Below, you talk about doing this without performance implications. Well,
that loop is one that you've added. :-)

> > Sorry, but if this design was discussed at some point, then I missed it. I
> > would have like to comment before you did all this work :-(
> 
> This design was discussed off-line.  Like most commits here, code means
> more than talk, so I implemented it, assuming people would comment and
> change it as they saw fit.  I didn't doo all that much work, most of the
> work was debugging, not coding.  :-)

IMO, no code is better than code that follows a suboptimal design. If you
wanted comments, then posting a patch would be sufficient.
(just like Manoj explained)

> > It would seem that mod_include should hook the output chain in one of the
> > request processing hooks (which? dunno... post-translation so that it can
> > determine whether include processing is enabled, surely). The hook occurs
> > underneath ap_rput*(). No module would need to change to get the benefits
> > of mod_include.
> 
> But any module that wants to do what mod_include is doing would need to be
> seriously hacked instead.  That's a trade off.  I would MUCH rather see
> modules that are adding/modifying content not have to change too much,
> because these are much more common than modules that are generating
> content.

I don't believe that the content-generators need to be changed AT ALL.
They would continue to call ap_rput*() as they do now.

The content-processors would need to change the following kind of loop:

  content-handler:
    fd = open()
    while 1:
      data = read()
      if not data:
        break
      process_data(data)

  process_data(data):
    ...
    ap_rputs(monkeyed_output)
    ...

to:

  some-hook-function:
    register_processor(process_data)

  process_data(next_layer, data):
    ...
    ap_layer_puts(next_layer, monkeyed_output)
    ...

I don't see a big change in the content-processors either. They just
remove their file-read loop and register their processing function. The
end of the chain does whatever ap_rput*() does now. The default handler is
going to be grabbing the file and doing an ap_rwrite(), which is then
going to be capture by mod_include (rather than mod_include reading the
file).

> > Your sub-thread design for mod_include seems fine, but note that all
> > processors would need something similar. That general logic probably
> > belongs in src/main, with a registered callback to process blocks of data:
> 
> No, the sub-thread design is a hack just to get it to work.  The real way
> this should be done, is to re-write mod_include to deal well with
> streaming data coming from the BUFF structure.

We agree on this at least :-). This was the rest of my email comments.

> > Well... back to the -1. Should this change be backed out? Some portions of
> > it?
> 
> Please don't back any of it out until you have read my response to your
> criticisms, and some more people have looked over the code. I would also
> like the opportunity to finish the work, so people could look it over in
> it's entirety.

I definitely didn't want to back it out until we got a chance to comment
back and forth a bit. That would be grossly unfair to you.

My hope was that you would also agree that it could be done
differently/better and back out the changes. I'll do the backing out if
you wish, but I felt it would be best to give you the option.

Along its current path, I do not agree this should be finished.

> One of the reasons this change was committed, was to get people working on
> this problem.  Please, if you are going to back out this change, post a
> complete design for something else that works.

I have already posted my idea: ap_rput*() implements a set of hooks that
can be intercepted by registering a callback function. In concrete terms,
we implement an ap_iol that calls the callback when methods->write is
invoked. Each time a processor calls register_processor(), we push one of
these new BUFFs/IOLs onto the stack.

Performance implications? None. If there are no processors, then the "top
BUFF" is connection->client. If somebody inserts something into the chain,
then yes... you have overhead (which is a given!).

Note that each intermediate BUFF will probably buffer the output. This
will minimize the number of calls to the callback.

The processors will need to change from ap_rput*() to ap_bput*(), using
the BUFF that is passed to the callback.

Are there holes in this design? Will this not work? (you ask for complete;
I think this is complete, short of coding)

Thx,
-g

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



Mime
View raw message