Atomic_min for floats

General Usage
,
1

I’m getting unexpected behavior for atomic_min!(x::Atomic{T}, val::T) with floats. This function returns the old value of x. Works fine for Ints, but for Floats it returns the new value:

julia> E = Atomic{Float64}(10)
Atomic{Float64}(10.0)

julia> atomic_min!(E, 3.0)
3.0

julia> E
Atomic{Float64}(3.0)

If I change the code to use Int64 then atomic_min() returns the old value (10). What is going on?

2

Looks like this line should end in return old in order to be consistent with the documentation:

Edit: Of course atomic_add! and atomic_sub! should still return new, so the fix will be more than three bytes.

3

Thanks. I wonder why the atomic functions return values and not booleans? The way it is now I have to compare the return value with x to see it the value of x changed, which could produce a race condition if x is written to by another thread.

4

I think it returns the old value to AVOID a race condition. If you atomically retrieve the value then atomically change the value, another thread might have changed it in between those two calls. If the method returns the OLD value, then you know what value was replaced and you can do whatever checks you need to do.

5

But if the function returned a boolean then I would know whether the value was replaced without having to do any checks.

6

But in that case you won’t know what the old value is. Also, @pixel27 was just arguing that there’s no race which was what you claimed…

I also don’t think there’s any difference in the actual machine code so it’s only a question of who write that comparison.

7

For sure the machine code is the same. It’s just a matter of choosing a higher level API, and there probably isn’t one solution for all cases. I’m used to the GCC atomic builtin functions, such as __sync_bool_compare_and_swap

8

Right it’s not about race or not. You can get the answer you want without a race.

And the current one is the one solution for all cases since the other one can be implemented using this, but not the other way around…

And fwiw, there’s __sync_val_compare_and_swap and pretty much all the other sync built-in returns the value instead of a boolean. It doesn’t even make much sense to return a boolean anyway since this operation, unlike CAS, is basically always a write operation anyway…

1 Like
9

Did anybody open an issue about this, or should I do it?

10

It looks like no one’s opened an issue on this. I can do it.