request for comments: new atomic API

[Brian Pane <br...@cnet.com>]

The current apr_atomic API has a couple of limitations:
* The size of apr_atomic_t varies among platforms, which
  makes it hard to guarantee portability of applications
  that use this type to store large values.
  
* The arithmetic functions use apr_atomic_t, while the
  compare-and-swap function uses a mix of apr_uint32_t
  and long int.  Thus it's impossible to portably write
  code that uses, say, apr_atomic_dec() and apr_atomic_cas()
  on the same variable.

This patch presents a proposed new API in which the
atomic functions explicitly operate on 32-bit ints.

I'd like to add commit this new API (plus an implementation)
next week.  But if anyone has objections or suggested changes,
please let me know...

Thanks,
Brian

Re: request for comments: new atomic API

[Brian Pane <br...@cnet.com>]

gregames@apache.org wrote:

> Brian Pane wrote:
>
>> On Sat, 2003-09-13 at 07:29, Wan-Teh Chang wrote:
>>
>>> 1. It seems that you intentionally do not want the add32, sub32,
>>> and inc32 return a value.  Only the dec32 function returns a
>>> value, and that value can only be used as a boolean (zero or
>>> nonzero).
>>
>>
>>
>>> 2. There is no atomic exchange function that atomically sets
>>> the new value and returns the old value.  (This can be implemented
>>> with the cas32 function in your API, so people can work around the
>>> lack of the atomic exchange function.)
>>
>>
>>
>> The simple reason for both of these is that the design
>> follows the original APR atomic API, which had void return
>> codes for the add/inc calls and no exchange function.
>> It wouldn't be too difficult to add an apr_atomic_xchg32.
>> For the add/subtract/inc functions, it definitely would
>> be nice to have the original value as a return code.  The
>> only problem is that seems difficult to do this without
>> making the functions more time-consuming.
>
>
> I would add return values to all of them right off the bat.  It seemed 
> like every time we came up with a new use for APR atomics, we needed a 
> return value. 


Do you know of an efficient way to return either the old
or the new value while still maintining atomicity?  It can
be done by using CAS to replace the old value with the new
value, but I've been trying to move away from that design
because it adds more overhead.  It's easy--and reasonably
efficient--to set a return code based on whether the new
value is zero, but using logic like this:
   atomic add/sub/inc/dec operation;
   if (zero flag is set in CPU) {
     result = 0;
   }
   else {
     result = 1;
   }
But I haven't figured out a way to return the actual value
(either the old or the new value) without using locking
or a CAS operation.

Brian

Re: request for comments: new atomic API

[gr...@apache.org]

Brian Pane wrote:
> On Sat, 2003-09-13 at 07:29, Wan-Teh Chang wrote:
> 
>>1. It seems that you intentionally do not want the add32, sub32,
>>and inc32 return a value.  Only the dec32 function returns a
>>value, and that value can only be used as a boolean (zero or
>>nonzero).
> 
> 
>>2. There is no atomic exchange function that atomically sets
>>the new value and returns the old value.  (This can be implemented
>>with the cas32 function in your API, so people can work around the
>>lack of the atomic exchange function.)
> 
> 
> The simple reason for both of these is that the design
> follows the original APR atomic API, which had void return
> codes for the add/inc calls and no exchange function.
> It wouldn't be too difficult to add an apr_atomic_xchg32.
> For the add/subtract/inc functions, it definitely would
> be nice to have the original value as a return code.  The
> only problem is that seems difficult to do this without
> making the functions more time-consuming.

I would add return values to all of them right off the bat.  It seemed like 
every time we came up with a new use for APR atomics, we needed a return value.

The original APR atomics were modeled after FreeBSD I believe, and that 
implementation isn't very useful.  The last time I searched thru the FreeBSD 
kernel code, the core didn't use these atomics much at all, but rather spin 
locks based on i386 xchg for Intel.  That should tell us something.

Then there's this other popular open source OS which has a much more technically 
useful set of atomics which set return values, but sadly an incompatible 
licence.  They are heavily used in that kernel.

Greg

Re: request for comments: new atomic API

[Cliff Woolley <jw...@virginia.edu>]

On Sun, 14 Sep 2003, Garrett Rooney wrote:

> I'd say that it should be documented to include any memory barriers that
> are needed, I mean that is a portability concern, and APR is a
> portability library.  I see a lot of people assuming that the atomic API
> would take care of that for them anyway, so we might as well document
> that it does and remove any doubt.

Agreed, +1.

--Cliff

Re: request for comments: new atomic API

[Garrett Rooney <ro...@electricjellyfish.net>]

Brian Pane wrote:

>>3. Providing cas32 is risky.  It tempts people to build lock-free
>>data structures with it, but the necessary memory barriers are
>>difficult to use right, and few people even realize that memory
>>barriers are necessary on some multiprocessor systems.
> 
> 
> You're right; this is another aspect of the old API that
> we probably need to rethink.  In the past, apr_atomic_cas
> has implicitly included a memory barrier on many systems--
> e.g., it uses the lock prefix when doing a cmpxchg on x86--
> but this hasn't been guaranteed on all platforms.  I'm not
> sure which is the right solution: have the API guarantee
> read and write ordering within the CAS function, or provide
> a memory separate memory barrier function.  Any recommendations?

I'd say that it should be documented to include any memory barriers that 
are needed, I mean that is a portability concern, and APR is a 
portability library.  I see a lot of people assuming that the atomic API 
would take care of that for them anyway, so we might as well document 
that it does and remove any doubt.

-garrett

Re: request for comments: new atomic API

[Brian Pane <br...@cnet.com>]

On Sat, 2003-09-13 at 07:29, Wan-Teh Chang wrote:
> 1. It seems that you intentionally do not want the add32, sub32,
> and inc32 return a value.  Only the dec32 function returns a
> value, and that value can only be used as a boolean (zero or
> nonzero).

> 2. There is no atomic exchange function that atomically sets
> the new value and returns the old value.  (This can be implemented
> with the cas32 function in your API, so people can work around the
> lack of the atomic exchange function.)

The simple reason for both of these is that the design
follows the original APR atomic API, which had void return
codes for the add/inc calls and no exchange function.
It wouldn't be too difficult to add an apr_atomic_xchg32.
For the add/subtract/inc functions, it definitely would
be nice to have the original value as a return code.  The
only problem is that seems difficult to do this without
making the functions more time-consuming.

> 3. Providing cas32 is risky.  It tempts people to build lock-free
> data structures with it, but the necessary memory barriers are
> difficult to use right, and few people even realize that memory
> barriers are necessary on some multiprocessor systems.

You're right; this is another aspect of the old API that
we probably need to rethink.  In the past, apr_atomic_cas
has implicitly included a memory barrier on many systems--
e.g., it uses the lock prefix when doing a cmpxchg on x86--
but this hasn't been guaranteed on all platforms.  I'm not
sure which is the right solution: have the API guarantee
read and write ordering within the CAS function, or provide
a memory separate memory barrier function.  Any recommendations?

Thanks,
Brian

Re: request for comments: new atomic API

[Wan-Teh Chang <wc...@aol.com>]

1. It seems that you intentionally do not want the add32, sub32,
and inc32 return a value.  Only the dec32 function returns a
value, and that value can only be used as a boolean (zero or
nonzero).

2. There is no atomic exchange function that atomically sets
the new value and returns the old value.  (This can be implemented
with the cas32 function in your API, so people can work around the
lack of the atomic exchange function.)

3. Providing cas32 is risky.  It tempts people to build lock-free
data structures with it, but the necessary memory barriers are
difficult to use right, and few people even realize that memory
barriers are necessary on some multiprocessor systems.

Wan-Teh

Re: request for comments: new atomic API

[Brian Pane <br...@cnet.com>]

On Sun, 2003-09-14 at 06:20, David Reid wrote:
> As we're approaching the time when we do a 1.0 why not simply remove the old
> interface rather than simply deprecate them? We shouldn't carry more into
> 1.0 then we need to after all!

+1 from me

Brian

Re: request for comments: new atomic API

[David Reid <da...@jetnet.co.uk>]

As we're approaching the time when we do a 1.0 why not simply remove the old
interface rather than simply deprecate them? We shouldn't carry more into
1.0 then we need to after all!

Otherwise, anything that makes the api more consistent seems like a good
idea.

david
----- Original Message ----- 
From: "Brian Pane" <br...@cnet.com>
To: <de...@apr.apache.org>
Sent: Saturday, September 13, 2003 6:46 AM
Subject: request for comments: new atomic API


> The current apr_atomic API has a couple of limitations:
> * The size of apr_atomic_t varies among platforms, which
>   makes it hard to guarantee portability of applications
>   that use this type to store large values.
>
> * The arithmetic functions use apr_atomic_t, while the
>   compare-and-swap function uses a mix of apr_uint32_t
>   and long int.  Thus it's impossible to portably write
>   code that uses, say, apr_atomic_dec() and apr_atomic_cas()
>   on the same variable.
>
> This patch presents a proposed new API in which the
> atomic functions explicitly operate on 32-bit ints.
>
> I'd like to add commit this new API (plus an implementation)
> next week.  But if anyone has objections or suggested changes,
> please let me know...
>
> Thanks,
> Brian
>
>