Return-Path: Delivered-To: apmail-httpd-dev-archive@httpd.apache.org Received: (qmail 45930 invoked by uid 500); 20 Dec 2002 20:35:18 -0000 Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm Precedence: bulk Reply-To: dev@httpd.apache.org list-help: list-unsubscribe: list-post: Delivered-To: mailing list dev@httpd.apache.org Received: (qmail 45900 invoked from network); 20 Dec 2002 20:35:18 -0000 Subject: Re: [PATCH] Update to Brian's patch to allocate brigades out of the bucket allocator From: Brian Pane To: rbb@apache.org Cc: dev@httpd.apache.org, APR Development List In-Reply-To: References: Content-Type: text/plain Organization: Message-Id: <1040416244.1423.35.camel@desktop> Mime-Version: 1.0 X-Mailer: Ximian Evolution 1.2.0 Date: 20 Dec 2002 12:30:45 -0800 Content-Transfer-Encoding: 7bit X-Spam-Rating: daedalus.apache.org 1.6.2 0/1000/N On Fri, 2002-12-20 at 12:19, rbb@apache.org wrote: > As for this not working when the filter and handler are in different > threads, of course it will. As long as the buckets are copied into a > brigade that was allocated out of a pool that will still be alive when the > filter is called, this will work just fine. In practice, it's not feasible to copy the buckets into a brigade allocated from a new pool. The connection pool isn't generally suitable, as it may be a long time before it is cleaned up, especially in the case of a proxy. Creating a "response pool" will work, but that doubles the amount of memory that the server must devote to a given request (one pool for request, one for response). > > The change doesn't remove that ability at all. Both my patch > > and Bill's variant allow for a pool cleanup to destroy a brigade. > > Yes, but before it was mandatory, not it is optional. It can't be > optional, it will cause a memory leak. It's wildly inaccurate to say "it will cause a memory leak." A more correct characterization is, "if the brigade is not allocated from a pool, its deallocation is the responsibility of the application." Same semantics as every other non-pool-based allocation API ever developed. > The change isn't buying you anything at all, especially with Bill's > proposed patch. All filters must assume that the bucket_brigade was > allocated out of a pool, because otherwise they will be broken if it > wasn't. That's only true for Apache 2.0. For later releases, it may be not only unnecessary but inadvisable to have brigades allocated from pools. Once you've allocated a brigade from a pool, for example, any subsequent split of that brigade will alloc from the same pool. If the split happens in a filter that's running in thread 2 while the request's handler is still running in thread 1, the colliding apr_pallocs will segfault. We almost certainly won't want to add a mutex to apr_palloc, because it would ruin the performance of lots of apps. But if the brigade allocations are done from a bucket allocator, it's quite feasible to add a mutex (or possibly a spinlock) into the bucket allocator without compromising performance. Brian