apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Justin Erenkrantz <jerenkra...@ebuilt.com>
Subject Re: freelists: a slightly different approach
Date Wed, 26 Sep 2001 22:23:45 GMT
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


Mime
View raw message