Re: GAtomic int finals (#63621)

[Tim Janik <timj gtk org>]

On Fri, 20 Feb 2004, Sebastian Wilhelmi wrote:

> Hi Tim,

> > (1) is unlikely to work out properly (complexity, portability, hard to debug,
> > requires knowledge about multiple architectures),
> > (2) requires an ordinary getter that is used exclusively for access, so
> > an rmb can be added if required by a platform.
>
> Ah, I see, you get to the same conclusion. Actually it might be wise to
> export a memory barrier (a full one, further optimizing in read/write
> seems to be too much) for glib's internal use. We should however not
> export it. That would allow to replace the current lightly crude "double
> checked locking" check in configure.in by proper memory barriers in
> gthread.c.

i don't think a memory barrier for glibs internal use is a good idea.
for one, coding atomic ints for glib shouldn't be anymore complicated than
coding them outside glib ;) and for another, if a getter solves our memory
barrier problems why export one then? (let alone that a getter can come with
a comparatively cheap rmb in cnstrast to a fat rmb+wmb)

> > since a getter can be a simple inlined read, it doesn't cost us anything
> > now (except API and mandatory use), so i strongly opt for adding it.
>
> Ok, new patch on the way.
>
> > on to the API names.
>
> Now it gets interesting. You can't argue taste....
>
> > > I've renamed g_atomic_int_exchange_and_add to
> > > g_atomic_int_get_and_add. It's shorter and adheres more to the GLib
> > > naming scheme and is more correct (Nothing is exchanged here).
> >
> > i don't think g_atomic_int_get_and_add() is an improvement. exchange_and_add()
> > is a pretty established name, we should stick with that. and in contrast to
> > get_and_add(), it tells me that the value returned is the int *before* the
> > add, not after. (so in effect, *i* do see what's exchanged here ;).
>
> Doesn't sound very convincing. We need an arbiter. Let Owen decide.

ok, will prompt him.

> > the API involved with atomic operations should be kept as small as possible,
> > that way it stays simple and avoids confusion about what primitives to use.
> > that is, we shouldn't provide g_atomic_int_inc() and g_atomic_int_dec_and_test().
>
> I would really like to keep both, even if they are one-liners. They make
> the reference counting code easier to read.

the gnits i really have with your variants are:
- #define g_atomic_int_inc(atomic) (g_atomic_int_add ((atomic), 1))
  i pretty much know that inc amounts to add(,1) so i don't see a clear advantage here
- #define g_atomic_int_dec_and_test(atomic) (g_atomic_int_get_and_add ((atomic), -1) == 1)
  a) for this, i'll have to lookup your definition everytime i use it, to
     figure whether you test for ==1 or >1
  b) looking at how the stl does refcounting for strings, they support a -1 count
     which means orphaned, can-be-freed, cant-be-referenced. i could quite possibly
     use something like that for GObject, in that light, get_and_add(,-1)==1 becomes
     useless, if not confusing.

yosh is in favour of these also, owen?


> > with the above 3 issues sorted out, the patch is ready to go into CVS, making
> > way for a variety of other glib improvements/optimizations ;)
>
> Making way for convoluted ways to shoot in our feet.
>
> Bye,
> Sebastian

---
ciaoTJ