httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Stoddard" <b...@wstoddard.com>
Subject [PATCH] mod_mem_cache using apr_atomic_dec()
Date Tue, 12 Mar 2002 21:57:12 GMT
I hesitate to commit the because I am not sure if apr_atomic_dec will be portable and
usable on enough OS/hardware combinations. mod_mem_cache could have several thousand
entries, so an apr_atomic_dec() that uses a lock per protected variable would not be too
cool...

If any apr_atomic_* implementations have hardware dependencies (ie, the implementation
explicitly exploits hardware features via assembly language calls for instance),
supporting the atomic operations in APR could become a real nightmare.  APR compiled on
one machine may not run on another machine even if the OS level of the two machines is
identical. Most gnarley...

Comments?

Bill

Index: mod_mem_cache.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_mem_cache.c,v
retrieving revision 1.32
diff -u -r1.32 mod_mem_cache.c
--- mod_mem_cache.c 12 Mar 2002 03:00:35 -0000 1.32
+++ mod_mem_cache.c 12 Mar 2002 21:44:55 -0000
@@ -61,6 +61,7 @@
 #include "mod_cache.h"
 #include "ap_mpm.h"
 #include "apr_thread_mutex.h"
+#include "apr_atomic.h"

 #if !APR_HAS_THREADS
 #error This module does not currently compile unless you have a thread-capable APR.
Sorry!
@@ -75,10 +76,6 @@
  * malloc/free rather than pools to manage their storage requirements.
  */

-/*
- * XXX Introduce a type field that identifies whether the cache obj
- * references malloc'ed or mmap storage or a file descriptor
- */
 typedef enum {
     CACHE_TYPE_FILE = 1,
     CACHE_TYPE_HEAP,
@@ -194,27 +191,17 @@
 static apr_status_t decrement_refcount(void *arg)
 {
     cache_object_t *obj = (cache_object_t *) arg;
-    mem_cache_object_t *mobj = (mem_cache_object_t*) obj->vobj;

-    if (sconf->lock) {
-        apr_thread_mutex_lock(sconf->lock);
-    }
-    obj->refcount--;
-    /* If the object is marked for cleanup and the refcount
-     * has dropped to zero, cleanup the object
-     */
-    if ((obj->cleanup) && (!obj->refcount)) {
-        cleanup_cache_object(obj);
-    }
-    if (sconf->lock) {
-        apr_thread_mutex_unlock(sconf->lock);
+    if (!apr_atomic_dec(&obj->refcount)) {
+        if (obj->cleanup) {
+            cleanup_cache_object(obj);
+        }
     }
     return APR_SUCCESS;
 }
 static apr_status_t cleanup_cache_mem(void *sconfv)
 {
     cache_object_t *obj;
-    mem_cache_object_t *mobj;
     apr_hash_index_t *hi;
     mem_cache_conf *co = (mem_cache_conf*) sconfv;

@@ -222,18 +209,15 @@
         return APR_SUCCESS;
     }

-    /* Iterate over the frag hash table and clean up each entry */
+    /* Iterate over the cache and clean up each entry */
     if (sconf->lock) {
         apr_thread_mutex_lock(sconf->lock);
     }
     for (hi = apr_hash_first(NULL, co->cacheht); hi; hi=apr_hash_next(hi)) {
         apr_hash_this(hi, NULL, NULL, (void **)&obj);
         if (obj) {
-            mobj = (mem_cache_object_t *) obj->vobj;
-            if (obj->refcount) {
-                obj->cleanup = 1;
-            }
-            else {
+            obj->cleanup = 1;
+            if (!apr_atomic_dec(&obj->refcount)) {
                 cleanup_cache_object(obj);
             }
         }
@@ -327,6 +311,8 @@
     /* Reference mem_cache_object_t out of cache_object_t */
     obj->vobj = mobj;
     mobj->m_len = len;
+    obj->complete = 0;
+    obj->refcount = 1;

     /* Place the cache_object_t into the hash table.
      * Note: Perhaps we should wait to put the object in the
@@ -339,8 +325,6 @@
      * rather than after the cache object has been completely built and
      * initialized...
      * XXX Need a way to insert into the cache w/o such coarse grained locking
-     * XXX Need to enable caching multiple cache objects (representing different
-     * views of the same content) under a single search key
      */
     if (sconf->lock) {
         apr_thread_mutex_lock(sconf->lock);
@@ -352,14 +336,6 @@
         apr_hash_set(sconf->cacheht, obj->key, strlen(obj->key), obj);
         sconf->object_cnt++;
         sconf->cache_size += len;
-        /* Call a cleanup at the end of the request to garbage collect
-         * a partially completed (aborted) cache update.
-         */
-        obj->complete = 0;
-        obj->refcount = 1;
-        obj->cleanup = 1;
-        apr_pool_cleanup_register(r->pool, obj, decrement_refcount,
-                                  apr_pool_cleanup_null);
     }
     if (sconf->lock) {
         apr_thread_mutex_unlock(sconf->lock);
@@ -374,6 +350,13 @@
         return DECLINED;
     }

+    /* Set the cleanup flag and register the cleanup to cleanup
+     * the cache_object_t when the cache load fails.
+     */
+    obj->cleanup = 1;
+    apr_pool_cleanup_register(r->pool, obj, decrement_refcount,
+                              apr_pool_cleanup_null);
+
     /* Populate the cache handle */
     h->cache_obj = obj;
     h->read_body = &read_body;
@@ -400,8 +383,8 @@
                                           APR_HASH_KEY_STRING);

     if (obj) {
-        mem_cache_object_t *mobj = (mem_cache_object_t *) obj->vobj;
         if (obj->complete) {
+            /* Refcount increment MUST be made under protection of the lock */
             obj->refcount++;
             apr_pool_cleanup_register(r->pool, obj, decrement_refcount,
apr_pool_cleanup_null);
         }
@@ -431,30 +414,39 @@

 static int remove_entity(cache_handle_t *h)
 {
-    cache_object_t *obj = h->cache_obj;
+    cache_object_t *obj;

+    /* Remove the object from the cache */
     if (sconf->lock) {
         apr_thread_mutex_lock(sconf->lock);
     }
-    obj = (cache_object_t *) apr_hash_get(sconf->cacheht, obj->key,
+    obj = (cache_object_t *) apr_hash_get(sconf->cacheht, h->cache_obj->key,
                                           APR_HASH_KEY_STRING);
     if (obj) {
         mem_cache_object_t *mobj = (mem_cache_object_t *) obj->vobj;
         apr_hash_set(sconf->cacheht, obj->key, strlen(obj->key), NULL);
         sconf->object_cnt--;
         sconf->cache_size -= mobj->m_len;
-        if (obj->refcount) {
-            obj->cleanup = 1;
-        }
-        else {
-            cleanup_cache_object(obj);
-        }
-        h->cache_obj = NULL;
     }
     if (sconf->lock) {
         apr_thread_mutex_unlock(sconf->lock);
-    }
-
+    }
+
+    /* If the object was removed from the cache prior to this function being
+     * called, then obj == NULL. Reinit obj with the object being cleaned up.
+     * Note: This code assumes that there is at most only one object in the
+     * cache per key.
+     */
+    obj = h->cache_obj;
+
+    /* Set cleanup to ensure decrement_refcount cleans up the obj if it
+     * is still being referenced by another thread
+     */
+    obj->cleanup = 1;
+    if (!apr_atomic_dec(&obj->refcount)) {
+        cleanup_cache_object(obj);
+    }
+    h->cache_obj = NULL;
     return OK;
 }
 static apr_status_t serialize_table(cache_header_tbl_t **obj,
@@ -529,7 +521,7 @@
      * apr_hash function.
      */

-    /* First, find the object in the cache */
+    /* Find and remove the object from the cache */
     if (sconf->lock) {
         apr_thread_mutex_lock(sconf->lock);
     }
@@ -540,19 +532,22 @@
         apr_hash_set(sconf->cacheht, key, APR_HASH_KEY_STRING, NULL);
         sconf->object_cnt--;
         sconf->cache_size -= mobj->m_len;
-        if (obj->refcount) {
-            obj->cleanup = 1;
-        }
-        else {
-            cleanup_cache_object(obj);
-        }
+        /* Refcount increment MUST be made under protection of the lock */
+        obj->refcount++;
     }
     if (sconf->lock) {
         apr_thread_mutex_unlock(sconf->lock);
     }

-    if (!obj) {
-        return DECLINED;
+    /* Cleanup the cache object */
+    if (obj) {
+        /* Set cleanup to ensure decrement_refcount cleans up the obj if it
+         * is still being referenced by another thread
+         */
+        obj->cleanup = 1;
+        if (!apr_atomic_dec(&obj->refcount)) {
+            cleanup_cache_object(obj);
+        }
     }

     return OK;


Mime
View raw message