Mailing-List: contact new-httpd-help@apache.org; run by ezmlm
Precedence: bulk
Reply-To: new-httpd@apache.org
Date: Thu, 16 Aug 2001 02:11:42 -0700
From: Brian Pane <bpane@pacbell.net>
Subject: Re: Optimizing dir_merge()
To: new-httpd@apache.org
Message-id: <3B7B8E4E.5010305@pacbell.net>
MIME-version: 1.0
Content-type: text/plain; charset=us-ascii; format=flowed
Content-transfer-encoding: 7BIT
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.3) Gecko/20010801
References: <00d301c125ea$ea541b00$96c0b0d0@roweclan.net>
 <3B7B1CF8.1020003@pacbell.net> <02a601c125f3$f152a350$96c0b0d0@roweclan.net>
Status: O

William A. Rowe, Jr. wrote:

>From: "Brian Pane" <bpane@pacbell.net>
>Sent: Wednesday, August 15, 2001 8:08 PM
>
>
>>William A. Rowe, Jr. wrote:
>>
>>>Here's my take on the dir_merge patch I offered up.  Please review for sanity.
>>>
>>Thanks!  This looks clear and complete to me.
>>
>>There's one point at the end where I disagree, though it may be due to
>>a bad assumption on my part.  Here's the item in question:
>>
>>[...]
>>
>>>Caching Considerations
>>>----------------------
>>>
>>>The key point within Apache is; a given per-dir config cannot be trusted after a 
>>>subsequent merge_dir_configs callback in the same pool.
>>>
>>I wouldn't have drawn this conclusion.  If your per-dir config is being passed as
>>the 'base' to a subsequent merge_dir_config callback, it should be unharmed afterward
>>because the callback must treat the base as const.  It's okay for the callback to
>>use a very liberal definition of constness if needed for performance reasons; e.g.,
>>it can increment a reference count in the base in support of copy-on-write logic.
>>But from the perspective of the caller, passing a cached per-dir config as the
>>base to a merge_dir_configs callback shouldn't change the semantics of the data
>>in that per-dir config.
>>
>>What am I missing?
>>
>
>"... in the same pool."
>
>If the caller passes a different pool to merge_dir_configs, they can be certain that
>the base is untouched, as are all the members of base.
>
>The optimization I proposed is this;  if the caller passes the same pool (as dir_walk
>would, potentially many times), then the 'optimized' module is free to build upon it's
>earlier efforts, e.g. adding or removing elements, whatever.  So a typical request
>(I ment to put this in the docs...) is
>
The basic optimization makes a lot of sense: allow the merge_dir function
to overwrite the base in cases where we know it's safe to do so.

But I'd prefer to find some safer way to decide that it's safe to modify
the base, rather than checking for matching pools.  The matching-pools
heuristic works fine in directory_walk, as currently implemented.  But
there may be other circumstances where a pool match doesn't mean it's
safe to treat the base struct as non-const.  As an example, if we pre-merge
the static cases of per-dir configs during startup, the pre-merged config
structure for /foo will get passed as the base to the merge_dir function
to build the pre-merged config for /foo/bar.  In this scenario, the pool
for the base will be the same pool passed to the merge-dir function
(specifically, it will be the config pool), but the merge-dir function
shouldn't modify the base.

I suppose the safe, general solution would be to provide a means for
the caller of the merge-per-dir-config function to specify whether the
function may modify the base, e.g. by adding a flag parameter or creating
different callback hooks for the "const" and "non-const" cases.  But an
API change like that would be an extreme solution, to say the least.

--Brian