You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Cliff Woolley <cl...@yahoo.com> on 2001/09/26 06:05:33 UTC
freelists: a slightly different approach
Okay, I've been thinking about this a lot today. I think I've come up
with a better design for this freelist scheme. The problem with the
current scheme is that it's very implementation specific... for example,
changing from SMS to the pass-in-an-int-list-number scheme required
changing the API, and the new version of the API basically implies a lot
of information about how we're implementing the free lists. I think it
would be much better to do the following:
1) each thread that wants to use buckets must call a function called
something like "apr_bucket_freelist_init(thread_pool)" which returns a
pointer of some opaque type. Something like this:
apr_bucket_freelist *apr_bucket_freelist_init(apr_pool_t *thread_pool);
2) the thread must simply remember the returned pointer and pass that in
to all calls to _bucket_foo_create().
This has several advantages:
a) No child-wide initialization is required (eg, to allocate a static
array of size num_threads or whatever), because the buckets code itself
will never keep track of ALL of the apr_bucket_freelist's it has created.
It's up to each caller to remember its own.
b) Because the buckets don't have to keep a static array or any sort of
table of the apr_bucket_freelist's, the number of threads can be totally
dynamic with no pre-defined maximum number.
c) If we choose to implement freelists another way, all we have to do is
change struct apr_bucket_freelist, and the callers will be oblivious to
the change. If I'd done it this way initially, for example, it would have
allowed us to change from SMS to the internal freelist scheme with no API
change at all.
All apr_bucket_freelist_init() has to do is allocate an
apr_bucket_freelist out of the thread_pool and initialize it (which means
allocating an initial block and so on).
As for allocation block size, I'm thinking we might have to take Ryan's
suggestion to go back to having bucket types register themselves so that
we can accurately determine the maximum size needed. If we're going to do
that, sizeof(private_data_structs) should become one of the fields in the
apr_bucket_type_t. Bucket types that have no private data (eg eos,
immortal) would just stick a 0 in there. This is clearly the most
reliable way to do it, but in a way it's kind of a shame... I was so
thrilled to have gotten rid of that registration stuff the first time
around. If anybody has other suggestions, I'm all ears...
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: freelists: a slightly different approach
Posted by Ryan Bloom <rb...@covalent.net>.
On Friday 28 September 2001 10:33 am, Branko Čibej wrote:
> Ryan Bloom wrote:
> >I would suggest not using a thread ID. They are incedibly non-portable.
> >Windows uses HANDLEs instead of ints.
>
> Not exactly. Windows has both thread handles and thead IDs. Use
> GetCurrentThread to get a handle, and GetCurrentThreadId to get an int.
>
> And anyway, a hash key doesn't have to be an int. Any kind of unique
> object will do. I doubt there's a threading library that doesn't have
> some sort of unique thread identifier available.
I am saying that today's APR doesn't have integer based thread ID's on all
platforms, nor should it need to. I am also saying that using a hash is just
plain wrong for this. It will be faster, and easier with just a simple array.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: freelists: a slightly different approach
Posted by Branko Čibej <br...@xbc.nu>.
Ryan Bloom wrote:
>I would suggest not using a thread ID. They are incedibly non-portable.
>Windows uses HANDLEs instead of ints.
>
Not exactly. Windows has both thread handles and thead IDs. Use
GetCurrentThread to get a handle, and GetCurrentThreadId to get an int.
And anyway, a hash key doesn't have to be an int. Any kind of unique
object will do. I doubt there's a threading library that doesn't have
some sort of unique thread identifier available.
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
Re: freelists: a slightly different approach
Posted by Ryan Bloom <rb...@covalent.net>.
On Wednesday 26 September 2001 01:00 am, Greg Stein wrote:
> On Wed, Sep 26, 2001 at 12:03:55AM -0700, Justin Erenkrantz wrote:
> > What's wrong with an internal hashtable here keyed off of the
> > thread id (or whatever is returned by apr_os_thread_current)?
> > Internally, you can represent the apr_bucket_freelist* as you
> > describe (that sounds good) - so we can abstract all of that
> > away, but forcing this out to the caller seems like the wrong
> > direction to go. The hashtable makes it so that it is of
> > indeterminate size and the retrieval speed should be fast
> > enough for our purposes.
> >
> > Or, what am I missing? -- justin
>
> An internal thread id seems more than fine. We can always expose more
> details later on, as we find we need to fine tune things.
>
> That thread id would then give you an array of chains of free buckets. That
> array can be indexed by the bucket size, say rounded up to the nearest 8 or
> 16 bytes. Buckets over size N fall off to a separate structure to associate
> sizes with the chains. That gives us speed for "normal" buckets, yet allows
> us to handle arbitrary size buckets.
>
> I believe the optimal structure for the chain is a pair of items:
>
> * a free list of buckets, in a simple linked list
> * a block of free buckets, specified as ptr to current and N left
>
> The free list optimizes the "free bucket" operation to be a simple linking
> process.
>
> Allocating a bucket goes to the free list first, then to the block. Both
> are very fast operations. If none are available, then a new block is
> allocated and the ptr/N pair are initialized.
>
> By using the pair, we optimize the alloc-new-block operation: linking N
> buckets into the free list is expensive. Dropping the new allocation into a
> block is the fastest operation. But putting free buckets back into that
> block is expensive, so we add in the free list.
>
> A bucket can simply record a ptr to its chain.
>
>
> So... the hard part is figuring out which chain to use when allocating a
> bucket. Like I said above, I would suggest a simple mechanism (for
> starters) which maps the thread id to an array of chains. Note that this
> does not require any API changes. Heck, it doesn't even need an API change
> for setting up the structures -- not to the bucket allocs and not even the
> apr_bucket_alloc stuff.
I would suggest not using a thread ID. They are incedibly non-portable.
Windows uses HANDLEs instead of ints. This is why I originally suggested
using the c->id number, and letting Apache handle which thread is doing the
work. Other than that, what you said above is exactly what I've been saying all
along, so I have to agree with all of it.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: freelists: a slightly different approach
Posted by Cliff Woolley <cl...@yahoo.com>.
On Wed, 26 Sep 2001, Greg Stein wrote:
> An internal thread id seems more than fine. We can always expose more
> details later on, as we find we need to fine tune things.
See my response to Justin.
> That thread id would then give you an array of chains of free buckets. That
> array can be indexed by the bucket size, say rounded up to the nearest 8 or
> 16 bytes. Buckets over size N fall off to a separate structure to associate
> sizes with the chains. That gives us speed for "normal" buckets, yet allows
> us to handle arbitrary size buckets.
>
> I believe the optimal structure for the chain is a pair of items:
>
> * a free list of buckets, in a simple linked list
> * a block of free buckets, specified as ptr to current and N left
This is what we've been talking about doing all along.
> So... the hard part is figuring out which chain to use when allocating a
> bucket. Like I said above, I would suggest a simple mechanism (for starters)
The very simplest method is one which involves no extra computation at
all. :) Yes, it's an API change, but I honestly believe we're going to
have to change the API anyway.
> which maps the thread id to an array of chains. Note that this does not
> require any API changes. Heck, it doesn't even need an API change for
> setting up the structures -- not to the bucket allocs and not even the
> apr_bucket_alloc stuff.
Are you suggesting that the buckets themselves call
apr_os_thread_current() or something every time you create a bucket...
wow, that would get really expensive really fast. Or have I missed your
point?
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: freelists: a slightly different approach
Posted by Greg Stein <gs...@lyra.org>.
On Wed, Sep 26, 2001 at 12:03:55AM -0700, Justin Erenkrantz wrote:
>...
> Something strikes me as incorrect with this. First, it'd require
> initialization of this code per thread and then they'd have to store
> it globally so that all of the code that calls _bucket_foo_create()
> can pass it - but since threads typically share memory space with
> their sibling threads, they'll need to create the data structure
> themselves. And, this code will have to be duplicated by any
> user of buckets.
Yah :-(
> What's wrong with an internal hashtable here keyed off of the
> thread id (or whatever is returned by apr_os_thread_current)?
> Internally, you can represent the apr_bucket_freelist* as you
> describe (that sounds good) - so we can abstract all of that
> away, but forcing this out to the caller seems like the wrong
> direction to go. The hashtable makes it so that it is of
> indeterminate size and the retrieval speed should be fast
> enough for our purposes.
>
> Or, what am I missing? -- justin
An internal thread id seems more than fine. We can always expose more
details later on, as we find we need to fine tune things.
That thread id would then give you an array of chains of free buckets. That
array can be indexed by the bucket size, say rounded up to the nearest 8 or
16 bytes. Buckets over size N fall off to a separate structure to associate
sizes with the chains. That gives us speed for "normal" buckets, yet allows
us to handle arbitrary size buckets.
I believe the optimal structure for the chain is a pair of items:
* a free list of buckets, in a simple linked list
* a block of free buckets, specified as ptr to current and N left
The free list optimizes the "free bucket" operation to be a simple linking
process.
Allocating a bucket goes to the free list first, then to the block. Both are
very fast operations. If none are available, then a new block is allocated
and the ptr/N pair are initialized.
By using the pair, we optimize the alloc-new-block operation: linking N
buckets into the free list is expensive. Dropping the new allocation into a
block is the fastest operation. But putting free buckets back into that
block is expensive, so we add in the free list.
A bucket can simply record a ptr to its chain.
So... the hard part is figuring out which chain to use when allocating a
bucket. Like I said above, I would suggest a simple mechanism (for starters)
which maps the thread id to an array of chains. Note that this does not
require any API changes. Heck, it doesn't even need an API change for
setting up the structures -- not to the bucket allocs and not even the
apr_bucket_alloc stuff.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: freelists: a slightly different approach
Posted by Ryan Bloom <rb...@covalent.net>.
On Wednesday 26 September 2001 03:23 pm, Justin Erenkrantz wrote:
> On Wed, Sep 26, 2001 at 01:43:42PM -0700, Ryan Bloom wrote:
> > You can keep saying to use the thread ID, but it won't work. Not all
> > platforms use an int to reference a thread_id. We have a opaque
> > data type for threads for a reason. At least two of our platforms today
> > do not refer to thread_id's as ints, BeOS and Windows. Now, we may
> > be able to get an int out of the platform, but we don't do it today. We
> > should not force platforms to provide an int. This is what we did with
> > processes, and it was wrong there, it will be wrong with threads too.
> >
> > If you want to use an int, it must be provided by the application using
> > the buckets. In httpd's case, use the c->id. Create a function that
> > allocates the correct size of an array, and just pass in c->id. DONE!
>
> Who said it was an int? I never said that anywhere. I was
> specifically referring to apr_os_thread_t and the comparison function
> apr_os_thread_equal. Both of these should be implemented per OS and
> there is no assumption that it is an int or a directly comparable
> anywhere in my suggestion. It can be a int, int*, my_thread*
> - whatever. But, if apr_os_thread_equal does not compare the two
> opaque structures correctly, then that is a broken function.
>
> (I notice that Win32 doesn't implement apr_os_thread_equal - why?
> Does it not have the ability to compare two tids? Or, should we
> be calling GetCurrentThreadID() instead? Perhaps we should call
> both and store it in the apr_os_thread_t for Win32?)
>
> The key idea behind my pseudocode was that the uniqueness is
> hidden within apr-util.
If it isn't an int, then the whole idea is useless unless we use a hash.
A hash is the wrong solution, because there is an absolute cap to the
number of slots we need in the table of freelists. Also, each thread has
a unique identifier already, so we can easily just use a simple array. It
just so happens that in this case, it is easy to use an httpd assigned unique
identifier. That is re-using a variable, it isn't breaking an abstraction.
> > There is no reason for either a linked-list or hashtable. This should be
> > a simple array. We are talking about 1 pointer per slot in the array,
> > allocated once. Assume a 64-bit platform, for a worst case scenerio, and
> > 2000 threads per process, and you have 1600 bytes for the static> > allocation.
>
> No, I don't believe it should be an array because you are requiring
> the arrays to be created by the user of the buckets. I think that
> is completely bogus. The user of the bucket code shouldn't care
> diddly-squat about the fact that the bucket code implements a
> freelist. The fact that we implement any type of freelist
> shouldn't be exposed externally outside of the bucket code - it
> should remain hidden. I think the external API to the buckets
> must remain the same.
Please read all of my messages. I have said since the beginning that
there would be an API, apr_bucket_list_init (bad name, but nobody ever
lets me name things), which would allocate the array. If you want to allocate
a hash table, you will need the same general function, or you will have
an if in every allocate function, which is just broken.
> And, for obvious reasons (i.e. the bucket code has no idea how
> many threads are active at one time), it can't use a simple
> array. Therefore, a linked-list or hashtable are required so
> that it can have grow enough slots for all active threads that
> request buckets.
>
> I think you are blurring the distinction between APR and httpd.
> Yes, you could create an array full of opaque data structures in
> httpd and pass that in to the bucket functions - I am merely
> stating that I believe that is broken API. My suggestion was
> just one way to handle the case without having the freelist
> API exposed to the outside world. That requires an external
> form of identifying the caller - which is what I believe the
> apr_os_thread_t provides.
Justin, go back and read the messages I have written please.
I have asked to add a single function call to httpd. That is a setup
function for the buckets. I am not talking about allocating any
data in httpd at all.
> I'm beginning to wonder whether we even need per-thread
> freelists at all. Our pools don't have them. Take this
> conversation and extend it to pools (i.e. when you create
> a pool, you pass in a caller-defined freelist) seems even
> more disturbing. -- justin
I don't even know how to respond to this. Just about everybody
on this list has said that a caller defined freelist is incorrect.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: freelists: a slightly different approach
Posted by Ryan Bloom <rb...@covalent.net>.
On Friday 28 September 2001 12:24 pm, Justin Erenkrantz wrote:
> On Wed, Sep 26, 2001 at 04:51:11PM -0700, Brian Pane wrote:
> > I disagree. We typically don't make APR data structures responsible
> > for their own memory management; rather, most things require that the
> > caller provide an appropriate pool, because the application can make
> > better choices about threads and resource management. I think the
> > same pattern can be applied to buckets; the only exception is that
> > they need a free-list instead of a pool.
>
> Okay, thinking of as a pool places it in a different light for me.
> I was thinking of it as merely an extension to the bucket code
> rather than as something completely separate.
>
> Here's an off-the-wall suggestion, why can't the buckets simply
> use the connection pool for allocation? Anything that is allocated
> for a bucket in httpd must have a defined lifetime no greater than
> the socket that the connection was created in - I'm not sure I see
> any need for keeping buckets around *after* the socket has closed.
> Am I missing something? Why can't we just simply use pools again?
> I think Sander (?) posted a patch that explicitly frees a chunk of
> memory from the pool, so we should be able to use that to minimize
> starvation when a bucket is "freed."
Because over time, the number of buckets used will stabilize, and we will
see a performance improvement by not having to allocate the buckets each
time. Also, there is no way to get the conn pool in each bucket list. Also,
the conn pool is incorrect, because we can leak like a sieve if we do that.
Imagine a connection that has 100 requests for files on it. That can be handled
by two or three buckets, but if we allocate out of the conn pool, we will
create two to three hundred, because they won't be free'd between requests.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: freelists: a slightly different approach
Posted by Cliff Woolley <cl...@yahoo.com>.
On Fri, 28 Sep 2001, Justin Erenkrantz wrote:
> On Fri, Sep 28, 2001 at 12:34:37PM -0700, Aaron Bannert wrote:
> > This is what I was alluding to in my absurdly vague message early on in
> > this thread. What I failed to realize at the time was that we don't have
> > a good way of getting that pool/freelist pointer deep into our filter
> > APIs. Am I looking at this correctly now?
>
> Yup. The only thing I can think of is adding a "bucket_freelist"
> parameter to ap_new_connection - this would be similar to the ptrans
> pool (but not cleared like ptrans at the end of the connection).
> Then, use f->c->bucket_freelist as the parameter to the bucket_create
> functions.
Exactly what I've been suggesting all along.
(FirstBill, GregA, Jeff, David and I racked our brains over this almost
four months ago now and this was the best option we could come up with...
I'm still convinced it's the best way to go.)
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: freelists: a slightly different approach
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Fri, Sep 28, 2001 at 12:34:37PM -0700, Aaron Bannert wrote:
> This is what I was alluding to in my absurdly vague message early on in
> this thread. What I failed to realize at the time was that we don't have
> a good way of getting that pool/freelist pointer deep into our filter
> APIs. Am I looking at this correctly now?
Yup. The only thing I can think of is adding a "bucket_freelist"
parameter to ap_new_connection - this would be similar to the ptrans
pool (but not cleared like ptrans at the end of the connection).
Then, use f->c->bucket_freelist as the parameter to the bucket_create
functions. I dunno if I like that or not... But, that's the cleanest
solution I can see without having the buckets handle this all
internally (which no one else seems to like). -- justin
Re: freelists: a slightly different approach
Posted by Aaron Bannert <aa...@clove.org>.
On Fri, Sep 28, 2001 at 12:24:49PM -0700, Justin Erenkrantz wrote:
> On Wed, Sep 26, 2001 at 04:51:11PM -0700, Brian Pane wrote:
> > I disagree. We typically don't make APR data structures responsible
> > for their own memory management; rather, most things require that the
> > caller provide an appropriate pool, because the application can make
> > better choices about threads and resource management. I think the
> > same pattern can be applied to buckets; the only exception is that
> > they need a free-list instead of a pool.
>
> Okay, thinking of as a pool places it in a different light for me.
> I was thinking of it as merely an extension to the bucket code
> rather than as something completely separate.
>
> Here's an off-the-wall suggestion, why can't the buckets simply
> use the connection pool for allocation? Anything that is allocated
> for a bucket in httpd must have a defined lifetime no greater than
> the socket that the connection was created in - I'm not sure I see
> any need for keeping buckets around *after* the socket has closed.
> Am I missing something? Why can't we just simply use pools again?
> I think Sander (?) posted a patch that explicitly frees a chunk of
> memory from the pool, so we should be able to use that to minimize
> starvation when a bucket is "freed."
Because we want to optimize the freelist for use with bucket allocation,
and we can do that best if we try to reuse free'd buckets. I think the
hope is that we can come up with an algorithm that quickly reaches
a stable state over the lifetime of the thread (worker thread or prefork
child), which means it has to live outside of a particular connection pool
lifetime.
This is what I was alluding to in my absurdly vague message early on in
this thread. What I failed to realize at the time was that we don't have
a good way of getting that pool/freelist pointer deep into our filter
APIs. Am I looking at this correctly now?
-aaron
Re: freelists: a slightly different approach
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Wed, Sep 26, 2001 at 04:51:11PM -0700, Brian Pane wrote:
> I disagree. We typically don't make APR data structures responsible
> for their own memory management; rather, most things require that the
> caller provide an appropriate pool, because the application can make
> better choices about threads and resource management. I think the
> same pattern can be applied to buckets; the only exception is that
> they need a free-list instead of a pool.
Okay, thinking of as a pool places it in a different light for me.
I was thinking of it as merely an extension to the bucket code
rather than as something completely separate.
Here's an off-the-wall suggestion, why can't the buckets simply
use the connection pool for allocation? Anything that is allocated
for a bucket in httpd must have a defined lifetime no greater than
the socket that the connection was created in - I'm not sure I see
any need for keeping buckets around *after* the socket has closed.
Am I missing something? Why can't we just simply use pools again?
I think Sander (?) posted a patch that explicitly frees a chunk of
memory from the pool, so we should be able to use that to minimize
starvation when a bucket is "freed."
> The *only* reason we've gotten this far without per-thread
> free lists for pools is that most apr_palloc() calls in the
> httpd happen to be small enough to work without additional
> block allocations from the global free list. In general, I
> don't think it's a bad idea to provide an alternate API for
> pool creation that says, "give this pool and its children
> a private, non-mutex-protected free list."
That's what Sander's pool replacement patch does, IIRC.
-- justin
Re: freelists: a slightly different approach
Posted by Cliff Woolley <cl...@yahoo.com>.
On Wed, 26 Sep 2001, Brian Pane wrote:
> >The fact that we implement any type of freelist
> >shouldn't be exposed externally outside of the bucket code - it
> >should remain hidden. I think the external API to the buckets
> >must remain the same.
> >
>
> I disagree. We typically don't make APR data structures responsible
> for their own memory management; rather, most things require that the
> caller provide an appropriate pool, because the application can make
> better choices about threads and resource management. I think the
> same pattern can be applied to buckets; the only exception is that
> they need a free-list instead of a pool.
This is clearly the key to explaining why this is the right design.
Don't think of it as a bucket freelist. Just think of it as a highly
specialized pool. That's all it really is anyway. So what we're doing
here is exactly like what all of the rest of APR does. apr_initialize()
returns the global pool. The caller is responsible for hanging onto that
and passing it (or a child of it) in to any APR functions that needs to
allocate some space. It's _exactly_ the same.
[I'll reiterate my agreement that it should be a pointer to the allocation
object and not some arbitrary index int.]
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: freelists: a slightly different approach
Posted by Brian Pane <bp...@pacbell.net>.
Justin Erenkrantz wrote:
[...]
>>There is no reason for either a linked-list or hashtable. This should be a simple
>>array. We are talking about 1 pointer per slot in the array, allocated once.
>>Assume a 64-bit platform, for a worst case scenerio, and 2000 threads per
>>process, and you have 1600 bytes for the static allocation.
>>
>
>No, I don't believe it should be an array because you are requiring
>the arrays to be created by the user of the buckets. I think that
>is completely bogus. The user of the bucket code shouldn't care
>diddly-squat about the fact that the bucket code implements a
>freelist. The fact that we implement any type of freelist
>shouldn't be exposed externally outside of the bucket code - it
>should remain hidden. I think the external API to the buckets
>must remain the same.
>
I disagree. We typically don't make APR data structures responsible
for their own memory management; rather, most things require that the
caller provide an appropriate pool, because the application can make
better choices about threads and resource management. I think the
same pattern can be applied to buckets; the only exception is that
they need a free-list instead of a pool.
[...]
>I think you are blurring the distinction between APR and httpd.
>Yes, you could create an array full of opaque data structures in
>httpd and pass that in to the bucket functions - I am merely
>stating that I believe that is broken API.
>
I agree on this part; I'd much rather require the app to pass in
an apr_bucket_free_list* than some numeric ID that gets mapped to
a free list. If the app wants to keep its free lists in an array,
that's fine, but it's cleaner if the bucket allocation code only
sees the one relevant apr_bucket_free_list*.
[...]
>I'm beginning to wonder whether we even need per-thread
>freelists at all. Our pools don't have them. Take this
>conversation and extend it to pools (i.e. when you create
>a pool, you pass in a caller-defined freelist) seems even
>more disturbing. -- justin
>
The *only* reason we've gotten this far without per-thread
free lists for pools is that most apr_palloc() calls in the
httpd happen to be small enough to work without additional
block allocations from the global free list. In general, I
don't think it's a bad idea to provide an alternate API for
pool creation that says, "give this pool and its children
a private, non-mutex-protected free list."
--Brian
Re: freelists: a slightly different approach
Posted by "William A. Rowe, Jr." <wr...@covalent.net>.
From: "Justin Erenkrantz" <je...@ebuilt.com>
Sent: Wednesday, September 26, 2001 5:23 PM
> (I notice that Win32 doesn't implement apr_os_thread_equal - why?
> Does it not have the ability to compare two tids? Or, should we
> be calling GetCurrentThreadID() instead? Perhaps we should call
> both and store it in the apr_os_thread_t for Win32?)
It's bogus today, and we should NOT NOT NOT be adding garbage calls.
apr_thread_t will gain a thand that will be _the_ handle from the
create thread, and td will become the Thread ID.
Then thread_equal becomes simple ;)
Bill
Re: freelists: a slightly different approach
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Wed, Sep 26, 2001 at 01:43:42PM -0700, Ryan Bloom wrote:
> You can keep saying to use the thread ID, but it won't work. Not all
> platforms use an int to reference a thread_id. We have a opaque
> data type for threads for a reason. At least two of our platforms today
> do not refer to thread_id's as ints, BeOS and Windows. Now, we may
> be able to get an int out of the platform, but we don't do it today. We
> should not force platforms to provide an int. This is what we did with
> processes, and it was wrong there, it will be wrong with threads too.
>
> If you want to use an int, it must be provided by the application using
> the buckets. In httpd's case, use the c->id. Create a function that
> allocates the correct size of an array, and just pass in c->id. DONE!
Who said it was an int? I never said that anywhere. I was
specifically referring to apr_os_thread_t and the comparison function
apr_os_thread_equal. Both of these should be implemented per OS and
there is no assumption that it is an int or a directly comparable
anywhere in my suggestion. It can be a int, int*, my_thread*
- whatever. But, if apr_os_thread_equal does not compare the two
opaque structures correctly, then that is a broken function.
(I notice that Win32 doesn't implement apr_os_thread_equal - why?
Does it not have the ability to compare two tids? Or, should we
be calling GetCurrentThreadID() instead? Perhaps we should call
both and store it in the apr_os_thread_t for Win32?)
The key idea behind my pseudocode was that the uniqueness is
hidden within apr-util.
> There is no reason for either a linked-list or hashtable. This should be a simple
> array. We are talking about 1 pointer per slot in the array, allocated once.
> Assume a 64-bit platform, for a worst case scenerio, and 2000 threads per
> process, and you have 1600 bytes for the static allocation.
No, I don't believe it should be an array because you are requiring
the arrays to be created by the user of the buckets. I think that
is completely bogus. The user of the bucket code shouldn't care
diddly-squat about the fact that the bucket code implements a
freelist. The fact that we implement any type of freelist
shouldn't be exposed externally outside of the bucket code - it
should remain hidden. I think the external API to the buckets
must remain the same.
And, for obvious reasons (i.e. the bucket code has no idea how
many threads are active at one time), it can't use a simple
array. Therefore, a linked-list or hashtable are required so
that it can have grow enough slots for all active threads that
request buckets.
I think you are blurring the distinction between APR and httpd.
Yes, you could create an array full of opaque data structures in
httpd and pass that in to the bucket functions - I am merely
stating that I believe that is broken API. My suggestion was
just one way to handle the case without having the freelist
API exposed to the outside world. That requires an external
form of identifying the caller - which is what I believe the
apr_os_thread_t provides.
I'm beginning to wonder whether we even need per-thread
freelists at all. Our pools don't have them. Take this
conversation and extend it to pools (i.e. when you create
a pool, you pass in a caller-defined freelist) seems even
more disturbing. -- justin
Re: freelists: a slightly different approach
Posted by Ryan Bloom <rb...@covalent.net>.
On Wednesday 26 September 2001 12:21 pm, Justin Erenkrantz wrote:
> On Wed, Sep 26, 2001 at 02:42:14PM -0400, Cliff Woolley wrote:
> > bit alone might disqualify it.) Oh well, it wouldn't kill us to have a
> > static array in each MPM that's the appropriate size and to just use
> > that, indexed by the second half of AP_CHILD_THREAD_FROM_ID. As long as
> > they get stored somewhere in Apache and not in apr-util I'm happy.
>
> I'd rather see it stored in apr-util not httpd. =-/
>
> You're trying to store internal information about the buckets in
> httpd and I'm not a fan of that (opaque pointer or not). This code
> to store the freelists would have to be duplicated by anybody that
> uses buckets - I'd like to see flood use buckets and brigades. As
> I said, this just seems like an incorrect way to go about it.
>
> It also breaks all sorts of abstractions. IMHO, sending in a unique
> identifier (such as c->id in the case of httpd) sounds like a fair
> compromise though - that's iffy though. I'd rather see Greg's
> suggestions of a linked-list of arrays indexed by the thread id. I
> just think that all of this should all be hidden by the bucket code.
You can keep saying to use the thread ID, but it won't work. Not all
platforms use an int to reference a thread_id. We have a opaque
data type for threads for a reason. At least two of our platforms today
do not refer to thread_id's as ints, BeOS and Windows. Now, we may
be able to get an int out of the platform, but we don't do it today. We
should not force platforms to provide an int. This is what we did with
processes, and it was wrong there, it will be wrong with threads too.
If you want to use an int, it must be provided by the application using
the buckets. In httpd's case, use the c->id. Create a function that
allocates the correct size of an array, and just pass in c->id. DONE!
>
> apr_bucket_get_my_freelist()
> {
> #if APR_HAS_THREADS
> apr_os_thread_t tid = apr_os_thread_current();
This is a pointer or structure on some platforms. That simple thing
just broken the rest of this code. Stop trying to use apr_os_thread_t
in portable code. The os in the type means it isn't portable.
> for all elements in the internal linked-list
> search for free-list associated with tid by
> apr_os_thread_equal(tid, l->tid)
> if no match found in linked-list
> create a new freelist for our tid
> attach to the linked list
> return match
> #else
> only-one-free-list, so return it.
> #endif
> }
>
> Note that since we're basing it off our thread id, it is impossible
> to have any sort of collisions (threads aren't reentrant). Therefore,
> you don't need to lock the walking of the lists. (Actually with
> a linked-list implementation, you *may* actually have a small race
> condition as you attach the new freelist to the list and you get
> interrupted mid-statement and someone is at the very end of the
> list - ack.)
Nobody has talked about using a mutex around this code ever.
> I think a hashtable would be faster as you wouldn't have to do
> a linear search and removes the race condition above, but
> whatever - people seem to think hashes are slow. FWIW, I'm
> picturing threads in the 100s or 1000s per process as the eventual
> typical case for httpd (flood already scales that high), so the
> balance shifts from a naive linked-list to a hashtable, IMHO.
There is no reason for either a linked-list or hashtable. This should be a simple
array. We are talking about 1 pointer per slot in the array, allocated once.
Assume a 64-bit platform, for a worst case scenerio, and 2000 threads per
process, and you have 1600 bytes for the static allocation.
This works because of the way that MPMs assign the connection id. They
all take child_num * MAX_THREADS_PER_CHILD + thread_num. If the bucket
code always does a modulous to ensure that the number passed in is within the
array, we are perfectly safe, portable, and fast.
What is the problem with this solution? The only argument I have heard against
it, is that it requires an integer to be passed to the bucket. So what. That int
is unique identifier for the process. If we define the API to require a unique ID,
and don't refer to the thread_num, or to any information from httpd, we keep all
of the abstractions, and it all just works.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: freelists: a slightly different approach
Posted by Cliff Woolley <cl...@yahoo.com>.
On Wed, 26 Sep 2001, Justin Erenkrantz wrote:
[I'll respond to the rest of this later, I'm late for class]
> apr_bucket_get_my_freelist()
> {
> #if APR_HAS_THREADS
> apr_os_thread_t tid = apr_os_thread_current();
> for all elements in the internal linked-list
> search for free-list associated with tid by
> apr_os_thread_equal(tid, l->tid)
> if no match found in linked-list
> create a new freelist for our tid
> attach to the linked list
> return match
> #else
> only-one-free-list, so return it.
> #endif
> }
>
This isn't even close to optimal. Here are just a few issues to think
about:
(a) apr_os_thread_current() might be expensive.
(b) Even if it's not, if you allocate fifteen buckets in a single filter,
you just asked the OS fifteen separate times in rapid succession for
the same information.
(c) There's no need for a search here. Any of the proposals involving
an array or some other kind of storage where a thread gets its own
slot per se will be much much faster. Constant time, not linear, not
even pseudo-constant like hashes. No locking, ever.
(d) What about prefork? Prefork can very easily have APR_HAS_THREADS,
but it doesn't need to do all this gobbledy gook. apr-util has no
way to know that it has threads but only one of them will ever use
this subsystem. If we use one of the other methods, it doesn't have
to know, because all of the freelist instances are independent.
Looking up the freelist for a single-threaded app would be the
essentially the same as it looking up for an app with dynamic numbers
of threads.
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: freelists: a slightly different approach
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Wed, Sep 26, 2001 at 02:42:14PM -0400, Cliff Woolley wrote:
> bit alone might disqualify it.) Oh well, it wouldn't kill us to have a
> static array in each MPM that's the appropriate size and to just use that,
> indexed by the second half of AP_CHILD_THREAD_FROM_ID. As long as they
> get stored somewhere in Apache and not in apr-util I'm happy.
I'd rather see it stored in apr-util not httpd. =-/
You're trying to store internal information about the buckets in
httpd and I'm not a fan of that (opaque pointer or not). This code
to store the freelists would have to be duplicated by anybody that
uses buckets - I'd like to see flood use buckets and brigades. As
I said, this just seems like an incorrect way to go about it.
It also breaks all sorts of abstractions. IMHO, sending in a unique
identifier (such as c->id in the case of httpd) sounds like a fair
compromise though - that's iffy though. I'd rather see Greg's
suggestions of a linked-list of arrays indexed by the thread id. I
just think that all of this should all be hidden by the bucket code.
apr_bucket_get_my_freelist()
{
#if APR_HAS_THREADS
apr_os_thread_t tid = apr_os_thread_current();
for all elements in the internal linked-list
search for free-list associated with tid by
apr_os_thread_equal(tid, l->tid)
if no match found in linked-list
create a new freelist for our tid
attach to the linked list
return match
#else
only-one-free-list, so return it.
#endif
}
Note that since we're basing it off our thread id, it is impossible
to have any sort of collisions (threads aren't reentrant). Therefore,
you don't need to lock the walking of the lists. (Actually with
a linked-list implementation, you *may* actually have a small race
condition as you attach the new freelist to the list and you get
interrupted mid-statement and someone is at the very end of the
list - ack.)
I think a hashtable would be faster as you wouldn't have to do
a linear search and removes the race condition above, but
whatever - people seem to think hashes are slow. FWIW, I'm
picturing threads in the 100s or 1000s per process as the eventual
typical case for httpd (flood already scales that high), so the
balance shifts from a naive linked-list to a hashtable, IMHO.
-- justin
Re: freelists: a slightly different approach
Posted by Ryan Bloom <rb...@covalent.net>.
On Wednesday 26 September 2001 11:42 am, Cliff Woolley wrote:
> On Wed, 26 Sep 2001, Ryan Bloom wrote:
> > > Will I get lynched if I further suggest putting this thing in the
> > > scoreboard? We've got our conn_rec->id that gives us a handy lookup
> > > into the scoreboard, so it seems a logical place to put it. Or it can
> > > just be a separate array, though the dimensions of that array will
> > > basically mirror the dimensions of the scoreboard anyway.
> >
> > Ummmmmm, how would that work? Will you be referencing pointers to
> > memory that isn't shared from shared memory? That seems a bit broken
> > to me.
>
> Welllllll, yeah. But I think I convinced myself it's a bad idea for
> performance reasons anyway. I'm not up to speed on how the locking
> mechanisms for the scoreboard work, but it'd suck for one thread to have
> to wait around on a scoreboard lock just because of this, when by
> definition it's an unshared pointer to unshared data. (I guess that last
> bit alone might disqualify it.) Oh well, it wouldn't kill us to have a
> static array in each MPM that's the appropriate size and to just use that,
> indexed by the second half of AP_CHILD_THREAD_FROM_ID. As long as they
> get stored somewhere in Apache and not in apr-util I'm happy.
There are no locks in the scoreboard at all. Because each thread has it's
own slot in the scoreboard, and it never tries to access another thread's spot,
this is perfectly safe.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: freelists: a slightly different approach
Posted by Cliff Woolley <cl...@yahoo.com>.
On Wed, 26 Sep 2001, Ryan Bloom wrote:
> > Will I get lynched if I further suggest putting this thing in the
> > scoreboard? We've got our conn_rec->id that gives us a handy lookup into
> > the scoreboard, so it seems a logical place to put it. Or it can just be
> > a separate array, though the dimensions of that array will basically
> > mirror the dimensions of the scoreboard anyway.
>
> Ummmmmm, how would that work? Will you be referencing pointers to
> memory that isn't shared from shared memory? That seems a bit broken
> to me.
Welllllll, yeah. But I think I convinced myself it's a bad idea for
performance reasons anyway. I'm not up to speed on how the locking
mechanisms for the scoreboard work, but it'd suck for one thread to have
to wait around on a scoreboard lock just because of this, when by
definition it's an unshared pointer to unshared data. (I guess that last
bit alone might disqualify it.) Oh well, it wouldn't kill us to have a
static array in each MPM that's the appropriate size and to just use that,
indexed by the second half of AP_CHILD_THREAD_FROM_ID. As long as they
get stored somewhere in Apache and not in apr-util I'm happy.
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: freelists: a slightly different approach
Posted by Ryan Bloom <rb...@covalent.net>.
On Wednesday 26 September 2001 10:13 am, Cliff Woolley wrote:
> On Wed, 26 Sep 2001, Ryan Bloom wrote:
> > That's fine.
>
> Will I get lynched if I further suggest putting this thing in the
> scoreboard? We've got our conn_rec->id that gives us a handy lookup into
> the scoreboard, so it seems a logical place to put it. Or it can just be
> a separate array, though the dimensions of that array will basically
> mirror the dimensions of the scoreboard anyway.
Ummmmmm, how would that work? Will you be referencing pointers to
memory that isn't shared from shared memory? That seems a bit broken
to me.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: freelists: a slightly different approach
Posted by Cliff Woolley <cl...@yahoo.com>.
On Wed, 26 Sep 2001, Ryan Bloom wrote:
> That's fine.
Will I get lynched if I further suggest putting this thing in the
scoreboard? We've got our conn_rec->id that gives us a handy lookup into
the scoreboard, so it seems a logical place to put it. Or it can just be
a separate array, though the dimensions of that array will basically
mirror the dimensions of the scoreboard anyway.
> Of course, you could just as easily pass an ID to the bucket API.
Certainly. But then the bucket API would have to map ID's to lists. The
nice part of the abstraction is that an apr_bucket_freelist* can be
anything... it can even be a plain old integer casted as a pointer if we
like. I believe it's more efficient and more flexible to have the thing
be an actual pointer to the freelist, so that's what I'm going to try.
But the key part is that as long as it's abstract, we can settle on an API
that fits our needs and then dink with the implementation till we're blue
in the face.
> Then, you keep the abstraction, because the bucket API just ends up
> accepting an ID to the create function. That ID ends up being an
> integer, but conceptually it is just a unique identifier. You could
> also create a simple function to return a unique ID to the caller, so
> that if the program didn't have their own ID, we could provide one.
That's basically what I'm saying we should do anyway with
apr_bucket_freelist_init() or whatever I called it... the caller really
doesn't much care what it receives and passes back. The only difference
is that with a generic "unique ID", the caller can make up their own.
That just means more work for us though. Whatever.
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: freelists: a slightly different approach
Posted by Ryan Bloom <rb...@covalent.net>.
On Wednesday 26 September 2001 08:23 am, Cliff Woolley wrote:
> On Wed, 26 Sep 2001, Ryan Bloom wrote:
> > Because this can all be easily accomplished with the connection ID
> > field that is already in the conn_rec.
>
> Well, then how's about we let Apache do the static array of
> apr_bucket_freelist*'s and use the connection ID to index into that?
> That way we keep the bucket API nice and clean and flexible and don't have
> to add another field to the conn_rec either. I can definitely live with
> that. What I don't like is the API design concept of "you pass me some
> index over which I have no control into some array that is otherwise
> totally internal to me."
That's fine. Of course, you could just as easily pass an ID to the bucket
API. Then, you keep the abstraction, because the bucket API just ends up
accepting an ID to the create function. That ID ends up being an integer,
but conceptually it is just a unique identifier. You could also create a simple
function to return a unique ID to the caller, so that if the program didn't have
their own ID, we could provide one.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: freelists: a slightly different approach
Posted by Cliff Woolley <cl...@yahoo.com>.
On Wed, 26 Sep 2001, Ryan Bloom wrote:
> Because this can all be easily accomplished with the connection ID
> field that is already in the conn_rec.
Well, then how's about we let Apache do the static array of
apr_bucket_freelist*'s and use the connection ID to index into that?
That way we keep the bucket API nice and clean and flexible and don't have
to add another field to the conn_rec either. I can definitely live with
that. What I don't like is the API design concept of "you pass me some
index over which I have no control into some array that is otherwise
totally internal to me."
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: freelists: a slightly different approach
Posted by Ryan Bloom <rb...@covalent.net>.
On Wednesday 26 September 2001 01:59 am, Cliff Woolley wrote:
> On Wed, 26 Sep 2001, Brian Pane wrote:
> > How about just adding an apr_bucket_freelist* to the conn_rec
> > and requiring each MPM to set it appropriately? Filters could
> > then reach the right free list via the ap_filter_t*, and most
> > other callbacks could reach it through the request_rec*, without
> > having to look up and hash the current thread ID for every bucket
> > alloc/split/free.
>
> Precisely!
Because this can all be easily accomplished with the connection ID field that
is already in the conn_rec.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: freelists: a slightly different approach
Posted by Cliff Woolley <cl...@yahoo.com>.
On Wed, 26 Sep 2001, Brian Pane wrote:
> How about just adding an apr_bucket_freelist* to the conn_rec
> and requiring each MPM to set it appropriately? Filters could
> then reach the right free list via the ap_filter_t*, and most
> other callbacks could reach it through the request_rec*, without
> having to look up and hash the current thread ID for every bucket
> alloc/split/free.
Precisely!
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: freelists: a slightly different approach
Posted by Brian Pane <bp...@pacbell.net>.
Justin Erenkrantz wrote:
[...]
>What's wrong with an internal hashtable here keyed off of the
>thread id (or whatever is returned by apr_os_thread_current)?
>Internally, you can represent the apr_bucket_freelist* as you
>describe (that sounds good) - so we can abstract all of that
>away, but forcing this out to the caller seems like the wrong
>direction to go. The hashtable makes it so that it is of
>indeterminate size and the retrieval speed should be fast
>enough for our purposes.
>
>Or, what am I missing? -- justin
>
How about just adding an apr_bucket_freelist* to the conn_rec
and requiring each MPM to set it appropriately? Filters could
then reach the right free list via the ap_filter_t*, and most
other callbacks could reach it through the request_rec*, without
having to look up and hash the current thread ID for every bucket
alloc/split/free.
--Brian
Re: freelists: a slightly different approach
Posted by Aaron Bannert <aa...@clove.org>.
On Wed, Sep 26, 2001 at 12:03:55AM -0700, Justin Erenkrantz wrote:
> Something strikes me as incorrect with this. First, it'd require
> initialization of this code per thread and then they'd have to store
> it globally so that all of the code that calls _bucket_foo_create()
> can pass it - but since threads typically share memory space with
> their sibling threads, they'll need to create the data structure
> themselves. And, this code will have to be duplicated by any
> user of buckets.
>
> What's wrong with an internal hashtable here keyed off of the
> thread id (or whatever is returned by apr_os_thread_current)?
> Internally, you can represent the apr_bucket_freelist* as you
> describe (that sounds good) - so we can abstract all of that
> away, but forcing this out to the caller seems like the wrong
> direction to go. The hashtable makes it so that it is of
> indeterminate size and the retrieval speed should be fast
> enough for our purposes.
>
> Or, what am I missing? -- justin
In trying to get my handle on this thing, I came up with a couple
questions/observations:
How is this different than an SMS specificially designed for buckets? From
my POV, take away the function pointers from SMS and we're left with a
memory allocator that has a definite lifetime (it takes on the lifetime
of the thread), can free as well as alloc, and is as optimized for the
particular problem of bucket allocation.
You can also view this as a special case of a pool, only one where there
is an explicit freelist that knows a little about the sizes of chunks
it will be getting back and redistributing. Hide those details behind
something like a thinner SMS and we've got it, no?
In either case you still have to create this structure and tie it to
some pool of the appropriate lifetime, which means it's going to have
to happen sometime after the thread's pool is created and before the
first connection is processed.
-aaron
Re: freelists: a slightly different approach
Posted by Cliff Woolley <cl...@yahoo.com>.
On Wed, 26 Sep 2001, Justin Erenkrantz wrote:
> Something strikes me as incorrect with this. First, it'd require
> initialization of this code per thread ... And, this code will have to
> be duplicated by any user of buckets.
That's going to be the case with any method you choose. Using a hash
table and the thread ID, you have to go one further... you have to
initialize per child [the hash table] and THEN per thread [each list].
> and then they'd have to store it globally so that all of the code that
> calls _bucket_foo_create() can pass it - but since threads typically
> share memory space with their sibling threads, they'll need to create
> the data structure themselves.
It's not that hard really... all threads already have private storage. It
can be kept on the stack or in a thread-private pool or anywhere. So it's
not really "global". The basic idea is to shove it into the conn_rec for
each connection or somewhere equally easily accessible.
> What's wrong with an internal hashtable here keyed off of the
> thread id (or whatever is returned by apr_os_thread_current)?
...
> and the retrieval speed should be fast enough for our purposes.
Two things. One, by using the thread ID and not just some arbitrary
pointer value, it ties us in to a certain set of possible implementations
[see below]. Two, I just think it will be wastefully slow if we have to
use a hash table. By wastefully slow, I mean that you're doing a _ton_ of
redundant computation. For each bucket you allocate, you might have to
rehash two or three times. Wow. That's no good. Maybe it would be 'fast
enough', but it could very easily be much, much faster. That's all I'm
saying.
> Internally, you can represent the apr_bucket_freelist* as you
> describe (that sounds good) - so we can abstract all of that
> away, but forcing this out to the caller seems like the wrong
> direction to go.
We're forcing it out to the caller any way around. Surely we don't want
to have to call apr_os_thread_current() every single time we allocate a
bucket. So we're already going to be storing the value in the caller.
The only difference is whether we hide what we're doing from the caller
(by making the apr_bucket_freelist* be the public interface) or allow the
caller a glimpse at what we're doing (by using the thread ID). It happens
that it's way more convenient for the buckets if we use the hiding
approach.
I think the problem here is that it it sounds to you like we're placing a
big burden on the caller. But if you stop to think about it, I think
you'll see that it's no more burden than any other method (in fact,
exactly the same amount). I guess I just need to code the damned thing up
so you can see. I was getting sick of rewriting this thing fifteen
different ways, but I'm now convinced that this is the right way to go, so
I guess I just have to go and do it if I'm going to convince you all as
well. =-)
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA
Re: freelists: a slightly different approach
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Wed, Sep 26, 2001 at 12:05:33AM -0400, Cliff Woolley wrote:
>
> Okay, I've been thinking about this a lot today. I think I've come up
> with a better design for this freelist scheme. The problem with the
> current scheme is that it's very implementation specific... for example,
> changing from SMS to the pass-in-an-int-list-number scheme required
> changing the API, and the new version of the API basically implies a lot
> of information about how we're implementing the free lists. I think it
> would be much better to do the following:
>
> 1) each thread that wants to use buckets must call a function called
> something like "apr_bucket_freelist_init(thread_pool)" which returns a
> pointer of some opaque type. Something like this:
>
> apr_bucket_freelist *apr_bucket_freelist_init(apr_pool_t *thread_pool);
>
> 2) the thread must simply remember the returned pointer and pass that in
> to all calls to _bucket_foo_create().
Something strikes me as incorrect with this. First, it'd require
initialization of this code per thread and then they'd have to store
it globally so that all of the code that calls _bucket_foo_create()
can pass it - but since threads typically share memory space with
their sibling threads, they'll need to create the data structure
themselves. And, this code will have to be duplicated by any
user of buckets.
What's wrong with an internal hashtable here keyed off of the
thread id (or whatever is returned by apr_os_thread_current)?
Internally, you can represent the apr_bucket_freelist* as you
describe (that sounds good) - so we can abstract all of that
away, but forcing this out to the caller seems like the wrong
direction to go. The hashtable makes it so that it is of
indeterminate size and the retrieval speed should be fast
enough for our purposes.
Or, what am I missing? -- justin