On Mon, 3 Sep 2001, Ryan Bloom wrote:
> On Monday 03 September 2001 18:49, Cliff Woolley wrote:
> > On Mon, 3 Sep 2001, Ryan Bloom wrote:
> > > I seriously dislike adding an SMS to every bucket. I believe we are
> > > much better off just creating a per-thread free bucket list. That
> > > keeps us from ever calling free while dealing with a request, and over
> > > time, we will stop calling malloc.
> >
> > Okay, let's examine this option for a minute. There are two ways to do
> > this: either the buckets code handles it or httpd handles it. If the
> > buckets code handles it, you have to do the thread lookups we talked about
> > before, which is bad. So that pretty much means it's up to the httpd to
> > handle it. So what's the easiest way to do that?
>
> The bucket code has to handle this. The easy way to do this, is to pass in
> an integer which corresponds to which bucket list to use.
Okay. Just so I'm sure we're on the same page before I go and code this
whole thing up, attached is a diff to apr_buckets.h that should make it
obvious what I'm proposing that we do.
Basically, I've taken all of the current _foo_create() functions and
renamed them to _foo_create_in() [name negotiable] and added an extra
parameter, which is just an int that tells you which list to use, as you
suggested. Then there's a new function called apr_bucket_list_find()
[name negotiable] that returns said int, basing it on the results of
apr_os_thread_current() or something like that. I've then created macros
for all of the _foo_create() functions of this form:
#define apr_bucket_foo_create(thisfoo) \
apr_bucket_foo_create_in(thisfoo,apr_bucket_list_find())
That way, if a caller wants to be efficient, he can call
apr_bucket_list_find() himself and store the value and use the
_foo_create_in() functions directly. The #define's let us preserve the
original API and migrate toward that approach more slowly and/or only
where really needed for performance. [How slowly probably depends upon
how expensive apr_os_thread_current() is.]
Does this seem reasonable?
--Cliff
--------------------------------------------------------------
Cliff Woolley
[EMAIL PROTECTED]
Charlottesville, VA
Index: apr_buckets.h
===================================================================
RCS file: /home/cvs/apr-util/include/apr_buckets.h,v
retrieving revision 1.118
diff -u -d -r1.118 apr_buckets.h
--- apr_buckets.h 2001/09/22 22:36:07 1.118
+++ apr_buckets.h 2001/09/23 21:12:12
@@ -1043,6 +1043,14 @@
APU_DECLARE_NONSTD(apr_status_t) apr_bucket_shared_copy(apr_bucket *a,
apr_bucket **b);
+/* ***** Bucket internal memory allocation ***** */
+/**
+ * Return the key into the hash table of freelists that should be used to
+ * find the freelist from which this bucket should be allocated. In most
+ * cases, it's just based on the current thread id.
+ * @return The freelist hash table key
+ */
+APU_DECLARE(int) apr_bucket_list_find(void);
/* ***** Functions to Create Buckets of varying types ***** */
/*
@@ -1061,7 +1069,9 @@
* coming from down the filter stack. All filters should flush at this point.
* @return The new bucket, or NULL if allocation failed
*/
-APU_DECLARE(apr_bucket *) apr_bucket_eos_create(void);
+#define apr_bucket_eos_create() \
+ apr_bucket_eos_create_in(apr_bucket_list_find())
+APU_DECLARE(apr_bucket *) apr_bucket_eos_create_in(int list);
/**
* Make the bucket passed in an EOS bucket. This indicates that there is no
@@ -1078,7 +1088,9 @@
* best we can do.
* @return The new bucket, or NULL if allocation failed
*/
-APU_DECLARE(apr_bucket *) apr_bucket_flush_create(void);
+#define apr_bucket_flush_create() \
+ apr_bucket_flush_create_in(apr_bucket_list_find())
+APU_DECLARE(apr_bucket *) apr_bucket_flush_create_in(int list);
/**
* Make the bucket passed in a FLUSH bucket. This indicates that filters
@@ -1095,8 +1107,11 @@
* @param nbyte The size of the data to insert.
* @return The new bucket, or NULL if allocation failed
*/
-APU_DECLARE(apr_bucket *) apr_bucket_immortal_create(const char *buf,
- apr_size_t nbyte);
+#define apr_bucket_immortal_create(buf,nbyte) \
+ apr_bucket_immortal_create_in(buf,nbyte,apr_bucket_list_find())
+APU_DECLARE(apr_bucket *) apr_bucket_immortal_create_in(const char *buf,
+ apr_size_t nbyte,
+ int list);
/**
* Make the bucket passed in a bucket refer to long-lived data
@@ -1115,8 +1130,11 @@
* @param nbyte The size of the data to insert.
* @return The new bucket, or NULL if allocation failed
*/
-APU_DECLARE(apr_bucket *) apr_bucket_transient_create(const char *buf,
- apr_size_t nbyte);
+#define apr_bucket_transient_create(buf,nbyte) \
+ apr_bucket_transient_create_in(buf,nbyte,apr_bucket_list_find())
+APU_DECLARE(apr_bucket *) apr_bucket_transient_create_in(const char *buf,
+ apr_size_t nbyte,
+ int list);
/**
* Make the bucket passed in a bucket refer to stack data
@@ -1143,9 +1161,11 @@
* If copy is zero then this return value can be ignored by passing a
NULL pointer.
* @return The new bucket, or NULL if allocation failed
*/
-APU_DECLARE(apr_bucket *) apr_bucket_heap_create(const char *buf,
- apr_size_t nbyte, int copy,
- apr_size_t *w);
+#define apr_bucket_heap_create(buf,nbyte,copy,w) \
+ apr_bucket_heap_create_in(buf,nbyte,copy,w,apr_bucket_list_find())
+APU_DECLARE(apr_bucket *) apr_bucket_heap_create_in(const char *buf,
+ apr_size_t nbyte, int
copy,
+ apr_size_t *w, int list);
/**
* Make the bucket passed in a bucket refer to heap data
* @param b The bucket to make into a HEAP bucket
@@ -1168,9 +1188,12 @@
* @param pool The pool the memory was allocated from
* @return The new bucket, or NULL if allocation failed
*/
-APU_DECLARE(apr_bucket *) apr_bucket_pool_create(const char *buf,
- apr_size_t length,
- apr_pool_t *pool);
+#define apr_bucket_pool_create(buf,length,pool) \
+ apr_bucket_pool_create_in(buf,length,pool,apr_bucket_list_find())
+APU_DECLARE(apr_bucket *) apr_bucket_pool_create_in(const char *buf,
+ apr_size_t length,
+ apr_pool_t *pool,
+ int list);
/**
* Make the bucket passed in a bucket refer to pool data
@@ -1193,9 +1216,12 @@
* @param length The number of bytes referred to by this bucket
* @return The new bucket, or NULL if allocation failed
*/
-APU_DECLARE(apr_bucket *) apr_bucket_mmap_create(apr_mmap_t *mm,
- apr_off_t start,
- apr_size_t length);
+#define apr_bucket_mmap_create(mm,start,length) \
+ apr_bucket_mmap_create_in(mm,start,length,apr_bucket_list_find())
+APU_DECLARE(apr_bucket *) apr_bucket_mmap_create_in(apr_mmap_t *mm,
+ apr_off_t start,
+ apr_size_t length,
+ int list);
/**
* Make the bucket passed in a bucket refer to an MMAP'ed file
@@ -1216,7 +1242,10 @@
* @param thissocket The socket to put in the bucket
* @return The new bucket, or NULL if allocation failed
*/
-APU_DECLARE(apr_bucket *) apr_bucket_socket_create(apr_socket_t *thissock);
+#define apr_bucket_socket_create(thissock) \
+ apr_bucket_socket_create_in(thissock,apr_bucket_list_find())
+APU_DECLARE(apr_bucket *) apr_bucket_socket_create_in(apr_socket_t *thissock,
+ int list);
/**
* Make the bucket passed in a bucket refer to a socket
* @param b The bucket to make into a SOCKET bucket
@@ -1231,7 +1260,10 @@
* @param thispipe The pipe to put in the bucket
* @return The new bucket, or NULL if allocation failed
*/
-APU_DECLARE(apr_bucket *) apr_bucket_pipe_create(apr_file_t *thispipe);
+#define apr_bucket_pipe_create(thispipe) \
+ apr_bucket_pipe_create_in(thispipe,apr_bucket_list_find())
+APU_DECLARE(apr_bucket *) apr_bucket_pipe_create_in(apr_file_t *thispipe,
+ int list);
/**
* Make the bucket passed in a bucket refer to a pipe
@@ -1251,10 +1283,12 @@
* while reading from this file bucket
* @return The new bucket, or NULL if allocation failed
*/
-APU_DECLARE(apr_bucket *) apr_bucket_file_create(apr_file_t *fd,
- apr_off_t offset,
- apr_size_t len,
- apr_pool_t *p);
+#define apr_bucket_file_create(fd,offset,len,p) \
+ apr_bucket_file_create_in(fd,offset,len,p,apr_bucket_list_find())
+APU_DECLARE(apr_bucket *) apr_bucket_file_create_in(apr_file_t *fd,
+ apr_off_t offset,
+ apr_size_t len,
+ apr_pool_t *p, int list);
/**
* Make the bucket passed in a bucket refer to a file
