You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Greg Ames <gr...@remulak.net> on 2002/03/15 19:35:13 UTC

Re: [PATCH] mod_mem_cache using apr_atomic_dec()

Bill Stoddard wrote:

(moved from dev@httpd to dev@apr)

> I hesitate to commit the because I am not sure if apr_atomic_dec will be portable and
> usable on enough OS/hardware combinations. 

as we discussed offline, you're right.  The return value for apr_atomic_dec
isn't defined, but as your code demonstrates, it's very useful for making the
cache more scaleable. 

> 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...

yeah, that's bad for binaries.  Seems like we would want to build them on the
most ancient version of the CPU architecture supported, or have #defines which
"dumb down" your build machine.

> -    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);
> +        }

nice...a concise lock free example of the "last guy out the door turns out the
lights" pattern.

However, this code won't work on Linux at the moment.  apr_atomic_dec invokes
the atomic_dec function which doesn't define a return value.  Linux does have an
atomic_dec_and_test which we could use.  But it returns true when the counter
becomes zero; Win32 returns the new value of the counter, so it is false when
the counter becomes zero.  It looks to me like Solaris behaves like Win32; dunno
about Netware.

FreeBSD doesn't seem to have userland atomic functions that return anything.  It
shouldn't be too hard to write an i386 asm function, but FreeBSD runs on other
chip families where I wouldn't feel comfortable with asm.  Perhaps we could
emulate a atomic_dec that tells you when the value reaches zero for non-i386
FreeBSD using a lock, like Ian's existing default functions.

Would it make sense to create another new API, apr_atomic_dec_and_test (or
whatever) which is defined to return something consistant?  I think Bill's code
demonstrates how useful it is to have such a function for reference counting. 
The Linux kernel uses both atomic_dec and atomic_dec_and_test.

We already have a return value from apr_atomic_cas;  I don't know that we need a
return value for the other atomics.

Greg

----------------------------------------------------------------------------

a quick hack to allow apr_atomic_dec to work on Linux for refcounts:

Index: srclib/apr/include/apr_atomic.h
===================================================================
RCS file: /cvs/apache/apr/include/apr_atomic.h,v
retrieving revision 1.15
diff -u -d -b -r1.15 apr_atomic.h
--- apr_atomic.h        2002/03/13 20:39:13     1.15
+++ apr_atomic.h        2002/03/15 17:36:13
@@ -117,6 +117,7 @@
 /**
  * decrement the atomic variable by 1
  * @param mem pointer to the atomic value
+ * @return false (0) if the value became 0; otherwise true
  */
 void apr_atomic_dec(volatile apr_atomic_t *mem);
 
@@ -158,7 +159,7 @@
 #define apr_atomic_t atomic_t
 
 #define apr_atomic_add(mem, val)     atomic_add(val,mem)
-#define apr_atomic_dec(mem)          atomic_dec(mem)
+#define apr_atomic_dec(mem)          !atomic_dec_and_test(mem)
 #define apr_atomic_inc(mem)          atomic_inc(mem)
 #define apr_atomic_set(mem, val)     atomic_set(mem, val)
 #define apr_atomic_read(mem)         atomic_read(mem)

Re: [PATCH] mod_mem_cache using apr_atomic_dec()

Posted by Ian Holsman <ia...@apache.org>.
Greg Ames wrote:
> Bill Stoddard wrote:
> 
> (moved from dev@httpd to dev@apr)
> 
> 
>>I hesitate to commit the because I am not sure if apr_atomic_dec will be portable and
>>usable on enough OS/hardware combinations. 
> 
> 
> as we discussed offline, you're right.  The return value for apr_atomic_dec
> isn't defined, but as your code demonstrates, it's very useful for making the
> cache more scaleable. 
> 
> 
>>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...
> 
> 
> yeah, that's bad for binaries.  Seems like we would want to build them on the
> most ancient version of the CPU architecture supported, or have #defines which
> "dumb down" your build machine.
> 
> 
>>-    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);
>>+        }
> 
> 
> nice...a concise lock free example of the "last guy out the door turns out the
> lights" pattern.
> 
> However, this code won't work on Linux at the moment.  apr_atomic_dec invokes
> the atomic_dec function which doesn't define a return value.  Linux does have an
> atomic_dec_and_test which we could use.  But it returns true when the counter
> becomes zero; Win32 returns the new value of the counter, so it is false when
> the counter becomes zero.  It looks to me like Solaris behaves like Win32; dunno
> about Netware.
> 
> FreeBSD doesn't seem to have userland atomic functions that return anything.  It
> shouldn't be too hard to write an i386 asm function, but FreeBSD runs on other
> chip families where I wouldn't feel comfortable with asm.  Perhaps we could
> emulate a atomic_dec that tells you when the value reaches zero for non-i386
> FreeBSD using a lock, like Ian's existing default functions.
> 


> Would it make sense to create another new API, apr_atomic_dec_and_test (or
> whatever) which is defined to return something consistant?  I think Bill's code
> demonstrates how useful it is to have such a function for reference counting. 
> The Linux kernel uses both atomic_dec and atomic_dec_and_test.

I think we should just modifiy the _dec function to return a value.
I'll patch this tonight if you don't beat me to it.

as for bsd.. I'm working on getting a freeBSD-current system which has a 
atomic_cmpset_int
which will allow us to implement the rest of them..

sorry for not fixing this earlier..
ian

> 
> We already have a return value from apr_atomic_cas;  I don't know that we need a
> return value for the other atomics.
> 
> Greg
> 
> ----------------------------------------------------------------------------
> 
> a quick hack to allow apr_atomic_dec to work on Linux for refcounts:
> 
> Index: srclib/apr/include/apr_atomic.h
> ===================================================================
> RCS file: /cvs/apache/apr/include/apr_atomic.h,v
> retrieving revision 1.15
> diff -u -d -b -r1.15 apr_atomic.h
> --- apr_atomic.h        2002/03/13 20:39:13     1.15
> +++ apr_atomic.h        2002/03/15 17:36:13
> @@ -117,6 +117,7 @@
>  /**
>   * decrement the atomic variable by 1
>   * @param mem pointer to the atomic value
> + * @return false (0) if the value became 0; otherwise true
>   */
>  void apr_atomic_dec(volatile apr_atomic_t *mem);
>  
> @@ -158,7 +159,7 @@
>  #define apr_atomic_t atomic_t
>  
>  #define apr_atomic_add(mem, val)     atomic_add(val,mem)
> -#define apr_atomic_dec(mem)          atomic_dec(mem)
> +#define apr_atomic_dec(mem)          !atomic_dec_and_test(mem)
>  #define apr_atomic_inc(mem)          atomic_inc(mem)
>  #define apr_atomic_set(mem, val)     atomic_set(mem, val)
>  #define apr_atomic_read(mem)         atomic_read(mem)
>