add support for indexing in @atomic macro

[kalmarek]

Following the discussion in #54642

Implemented:

• modifyindex_atomic!, swapindex_atomic!, replaceindex_atomic! for GenericMemory
• getindex_atomic, setindex_atomic!, setindexonce_atomic! for GenericMemory
• add support for references in @atomic macros
• add support for vararg indices in @atomic macros
• tests
• update docstrings with example usage
• [ ] update Atomics section of the manual (?)
• news

@oscardssmith @vtjnash

New @atomic transformations implemented here:

julia> @macroexpand (@atomic a[i1,i2])
:(Base.getindex_atomic(a, :sequentially_consistent, i1, i2))

julia> @macroexpand (@atomic order a[i1,i2])
:(Base.getindex_atomic(a, order, i1, i2))

julia> @macroexpand (@atomic a[i1,i2] = 2.0)
:(Base.setindex_atomic!(a, :sequentially_consistent, 2.0, i1, i2))

julia> @macroexpand (@atomic order a[i1,i2] = 2.0)
:(Base.setindex_atomic!(a, order, 2.0, i1, i2))

julia> @macroexpand (@atomicswap a[i1,i2] = 2.0)
:(Base.swapindex_atomic!(a, :sequentially_consistent, 2.0, i1, i2))

julia> @macroexpand (@atomicswap order a[i1,i2] = 2.0)
:(Base.swapindex_atomic!(a, order, 2.0, i1, i2))

julia> @macroexpand (@atomic a[i1,i2] += 2.0)
:((Base.modifyindex_atomic!(a, :sequentially_consistent, +, 2.0, i1, i2))[2])

julia> @macroexpand (@atomic order a[i1,i2] += 2.0)
:((Base.modifyindex_atomic!(a, order, +, 2.0, i1, i2))[2])

julia> @macroexpand (@atomiconce a[i1,i2] = 2.0)
:(Base.setindexonce_atomic!(a, :sequentially_consistent, :sequentially_consistent, 2.0, i1, i2))

julia> @macroexpand (@atomiconce o1 o2 a[i1,i2] = 2.0)
:(Base.setindexonce_atomic!(a, o1, o2, 2.0, i1, i2))

julia> @macroexpand (@atomicreplace a[i1,i2] (2.0=>3.0))
:(Base.replaceindex_atomic!(a, :sequentially_consistent, :sequentially_consistent, 2.0, 3.0, i1, i2))

julia> @macroexpand (@atomicreplace o1 o2 a[i1,i2] (2.0=>3.0))
:(Base.replaceindex_atomic!(a, o1, o2, 2.0, 3.0, i1, i2))

[oscardssmith]

other than the comment above this looks good! (it is missing an implementation of getindex_atomic which we need because the regular getindex doesn't have a place to pass an order)

[kalmarek]

@oscardssmith I'm not sure about the semantics of the suggested getindex_atomic. One can write a completely generic

function getindex_atomic(mem::GenericMemory, i::Int, order=default_access_order(mem))
    memref = memoryref(mem, i, @_boundscheck)
    return memoryrefget(memref, i, order, @_boundscheck)
end

but there's nothing atomic about the function, except the order argument. Is this what you meant?

[oscardssmith]

The point is just that (but probably restricted to AtomicMemory since

julia> m = Memory{Int}(undef, 5);

julia> Base.memoryrefget(memoryref(m, 1), :acquire, false)
ERROR: ConcurrencyViolationError("memoryrefget: non-atomic memory cannot be accessed atomically")
Stacktrace:
 [1] top-level scope
   @ REPL[3]:1

The reason we need this function is for the user to be able to provide a stricter order than :monotonic

[kalmarek]

@oscardssmith thanks for the clarification;

Alongside modifyindex_atomic!, swapindex_atomic! and replaceindex_atomic! I've also implemented

• getindex_atomic(::GenericMemory, i, ...)
• setindex_atomic!(::GenericMemory, i, ...) and
• setindexonce_atomic!(::GenericMemory, i, ...).

I added some converts (val isa T ? val : convert(T, val)::T) those *index methods and the support for indexing in @atomic macro with the semantics as in the opening comment.

Please have a look and comment!

I'll write/update docstrings once the code is approved.

[oscardssmith]

This looks great to me! We definitely need more tests (and review from @vtjnash to make sure that all of the default orders etc are correct).

[kalmarek]

the default orderings are the ones from the existing cases of @atomic macro.

[kalmarek]

@vtjnash could you have a look at the proposed changes?
In particular the choices of orderings for the @atomic macro.
Do we need even more tests?

@oscardssmith what kind of docs should be written?

To me the docstring of the atomic macro is already too long, asking rather for a paragraph in the atomics part of the manual.

[oscardssmith]

I think this does need some docs under the @atomic macro, e.g.

  @atomic m[i] = new
  @atomic m[i] += addend
  @atomic :release m[i] = new
  @atomic :acquire_release m[i] += addend

but I agree with you that the docstring is already pretty long, but I kind of think that it is necessary to at least list the transforms that the macro does.

[kalmarek]

@oscardssmith @vtjnash I've updated docstrings and added NEWS entry. Please let me know if something else needs to be done/changed (i.e. please review :).

To me it seems ready to merge!

[vchuravy]

Would be nice to check that Atomix.jl still works and can be used as a backwards compatible layer to get this functionality on older Julia versions.

[kalmarek]

@vchuravy I was actually thinking about spinning this functionality into a small package for julia-1.11, but merging into Atomix.jl would be even better. My only hesitation is that this package has not seen any activity in 2 years and was mostly developed by the brilliant tkf who left the community. Is it worth the effort?

[vchuravy]

Atomix is actively used and @maleadt or I have commit privileges.

[kalmarek]

@oscardssmith @vtjnash please let me know what else need to be done here.

@nsajko, since you have access to lables I think that needs news, needs docs and needs tests are satisfied and could be removed.

[oscardssmith]

IMO this is ready to merge, but I do want @vtjnash to have a last lock

[oscardssmith]

Thanks so much for doing this!

[vtjnash]

Awesome!