Document GenericMemory and AtomicMemory #54642
Conversation
oscardssmith
added
backport 1.11
base/genericmemory.jl
Outdated
| One-dimensional, fixed-size, dense array with elements of type `T`, where each element is | ||
| independently atomic when accessed, and cannot be set non-atomically. | ||
| For details, see [Atomic Operations](@ref man-atomic-operations) |
There was a problem hiding this comment.
what does it mean precisely in terms of julia code? man-atomic-operations deals with Threads.Atomic as well as @atomic macro. I presume these sentences relate to the latter. Does that mean that at_mem[3] = 5 is performed atomically (without adding @atomic in front? Same question for x = at_mem[3].
I've been waiting for using this already quite some time but it's really hard without any docs ;) e.g. can a non-isbits x be in a non-consistent state when read from at_mem::AtomicMemory (e.g. some other thread was modifying it at the same time)?
I presume that the whole point of AtomicMemory is that one does not need to use @atomic, but I'd be nice to have it stated in clear text here.
also a tangent question, but the next building block would be a CAS operation on an AtomicMemory, but neither this docstring nor man-atomic-operations mention it (except for Threads.atomic_cas! which deals only with Threads.Atomic).
There was a problem hiding this comment.
so the issue currently is that the @atomic macro hasn't been updated to support Atomic Memory, but once it is, it will allow you to write
@atomic :release mem[3]=2 for example.
There was a problem hiding this comment.
does that mean that @atomic macro will be obligatory for reading AtomicMemory? or that it would default to :monotonic? how about writing into AtomicMemory? and of course cas operations: will there be @atomiccas mem[3] 4=>2 ala Atomix.jl?
There was a problem hiding this comment.
reads default to monotonic. writes require an explicit order. not sure about CAS.
There was a problem hiding this comment.
@oscardssmith it would be great if this information were included in the docs!
There was a problem hiding this comment.
@vtjnash thanks for the info. The Manifesto was modified last time one year ago and doesn't even mention AtomicMemory so I was hesitant to rely on the document.
What I understand is that the plan is to implement
@atomic ord x[3]→Core.memoryrefget(memoryref(x, 3), ord, ...)@atomic ord x[3] = 4→Core.memoryrefset!(memoryref(x, 3), 4, ord, ...)@atomicswap ord x[3]→Core.memoryrefswap!(memoryref(x, 3), 4, ord, ...)@atomicreplace ord x[3] 3=>4→Core.memoryrefreplace!(memoryref(x, 3), 3, 4, ord, ...)
to extend the @atomic[...] macros? (So far these work only on atomic fields). I'd happily give it a try myself if that's the goal ;)
There was a problem hiding this comment.
also docstrings of Core.memoryref[...] functions mention often isatomic parameter which I infer to be the type parameter of AtomicMemory but isatomic sounds more like function (on Memory?) and is not mentioned at all in the docs elsewhere.
There was a problem hiding this comment.
I believe that's a remnant of an earlier version of the pr where we had an isatomic bool for the type parameter rather than a symbol kind. time to fix the docs more :)
There was a problem hiding this comment.
For CAS, we have Core.memoryrefreplace! and Core.memoryrefmodify! which are the implimentations of the (to be created modifyindex! and replaceindex! that function analogously to modifyfield! and replacefield! respectively.
There was a problem hiding this comment.
Also when we do this, we will want to give CUDA.jl a companion PR that updates https://github.com/JuliaGPU/CUDA.jl/blob/b07cf990bce048e106c7d767ae6003d574706407/src/device/intrinsics/atomics.jl to use the new names.
| const Generic = bitcast(Core.AddrSpace{CUDA}, 0) | ||
| const Global = bitcast(Core.AddrSpace{CUDA}, 1) |
There was a problem hiding this comment.
| const Generic = bitcast(Core.AddrSpace{CUDA}, 0) | |
| const Global = bitcast(Core.AddrSpace{CUDA}, 1) | |
| const Generic = Core.AddrSpace{CUDA}(0x00) | |
| const Global = Core.AddrSpace{CUDA}(0x01) |
This code doesn't work since a) bitcast is not public, and b) the integer must be a 1-byte integer for bitcast to work
There was a problem hiding this comment.
I think this was unresolved with the force push.
There was a problem hiding this comment.
oops. Will re-resolve once #54681 merges.
While writing docs in #54642, I became dis-satisfied with how `MemoryRef` currently works and since we are approaching the deadline for changing things around here, made this. Previously, we had the builtin `memoryref` which constructed a `GenericMemoryRef` from a `GenericMemory` or from a `GenericMemoryRef` and an offset. and then we defined constructors `GenericMemoryRef`, `MemoryRef` etc that provided a nicish interface around the intrinsic. The problem with this is that people who want to make a `GenericMemoryRef` don't care what kind of `GenericMemoryRef` they are making. That choice is defined by the `GemericMemory`. As such, I have switched it around so that now the intrinsic is named `memoryrefnew` which frees up `memoryref` to be the function that constructs the appropriate type of `GenericMemoryRef`. This could have been done purely on the Base side, but renaming the intrinsic seems worth it to me since Base/Core use the (new) `memoryref` a lot and it seems like we should make the experience the same for internal and external users rather than making `Base` have to work around a bad name.
oscardssmith
force-pushed
the
document-GenericMemory
branch
from
aed6011
to
756326d
Compare
|
|
||
| One-dimensional dense array with elements of type `T`. | ||
| Fixed-size [`DenseVector{T}`](@ref DenseVector). |
There was a problem hiding this comment.
| Fixed-size [`DenseVector{T}`](@ref DenseVector). | |
| A [`DenseVector{T}`](@ref DenseVector). Fixed-size, in the sense that the length of a value of this type doesn't change after construction. |
Maybe it'd be better to be more precise regarding "fixed-size"?
There was a problem hiding this comment.
I don't really see this is clearer. Is there an ambiguity with the original version? Would "constant length" be clearer?
There was a problem hiding this comment.
Perhaps the current version is OK. My motivation is to avoid confusion with the StaticArrays.jl-sense of "constant length", where the length is in the type domain.
Co-authored-by: Marek Kaluba <kalmar@mailbox.org>
Co-authored-by: Neven Sajko <s@purelymail.com>
Co-authored-by: Neven Sajko <s@purelymail.com>
oscardssmith
force-pushed
the
document-GenericMemory
branch
from
95cab81
to
9f7959f
Compare
|
review addressed. I believe this is ready to merge. |
|
Note that #54707 (comment) fixes the giant warning that this PR adds that basically says "sorry you can't use AtomicMemory yet", but this warning will likely exist in 1.11 since I don't think we want to backport that PR this late in the release. |
Closes #53854. After talking with @vtjnash, we are ready to commit to the `GenericMemory` interface. Sorry @nsajko that this took me so long to get around to. --------- Co-authored-by: Marek Kaluba <kalmar@mailbox.org> Co-authored-by: Neven Sajko <s@purelymail.com> (cherry picked from commit 589fd1a)
While writing docs in #54642, I became dis-satisfied with how `MemoryRef` currently works and since we are approaching the deadline for changing things around here, made this. Previously, we had the builtin `memoryref` which constructed a `GenericMemoryRef` from a `GenericMemory` or from a `GenericMemoryRef` and an offset. and then we defined constructors `GenericMemoryRef`, `MemoryRef` etc that provided a nicish interface around the intrinsic. The problem with this is that people who want to make a `GenericMemoryRef` don't care what kind of `GenericMemoryRef` they are making. That choice is defined by the `GemericMemory`. As such, I have switched it around so that now the intrinsic is named `memoryrefnew` which frees up `memoryref` to be the function that constructs the appropriate type of `GenericMemoryRef`. This could have been done purely on the Base side, but renaming the intrinsic seems worth it to me since Base/Core use the (new) `memoryref` a lot and it seems like we should make the experience the same for internal and external users rather than making `Base` have to work around a bad name. (cherry picked from commit fa038d9)
Backported PRs: - [x] #54361 <!-- [LBT] Upgrade to v5.9.0 --> - [x] #54474 <!-- Unalias source from dest in copytrito --> - [x] #54548 <!-- Fixes for bitcast bugs with LLVM 17 / opaque pointers --> - [x] #54191 <!-- make `AbstractPipe` public --> - [x] #53402 <!-- Add `jl_getaffinity` and `jl_setaffinity` --> - [x] #53356 <!-- Rename at-scriptdir project argument to at-script and search upwards for Project.toml --> - [x] #54545 <!-- typeintersect: fix incorrect innervar handling under circular env --> - [x] #54586 <!-- Set storage class of julia globals to dllimport on windows to avoid auto-import weirdness. Forward port of #54572 --> - [x] #54587 <!-- Accomodate for rectangular matrices in `copytrito!` --> - [x] #54617 <!-- CLI: Use `GetModuleHandleExW` to locate libjulia.dll --> - [x] #54605 <!-- Allow libquadmath to also fail as it is not available on all systems --> - [x] #54634 <!-- Fix trampoline assembly for build on clang 18 on apple silicon --> - [x] #54635 <!-- Aggressive constprop in trevc! to stabilize triangular eigvec --> - [x] #54645 <!-- ensure we set the right value to gc_first_tid --> - [x] #54554 <!-- make elsize public --> - [x] #54648 <!-- Construct LazyString in error paths for tridiag --> - [x] #54658 <!-- fix missing uuid check on extension when finding the location of an extension --> - [x] #54594 <!-- Switch to Pkg mode prompt immediately and load Pkg in the background --> - [x] #54669 <!-- Improve error message in inplace transpose --> - [x] #54671 <!-- Add boundscheck in bindingkey_eq to avoid OOB access due to data race --> - [x] #54672 <!-- make: Fix `sed` command for LLVM libraries with no symbol versioning --> - [x] #54624 <!-- more precise aliasing checks for SubArray --> - [x] #54679 <!-- 🤖 [master] Bump the Distributed stdlib from 6a07d98 to 6c7cdb5 --> - [x] #54604 <!-- Fix tbaa annotation on union selector bytes inside of structs --> - [x] #54690 <!-- Fix assertion/crash when optimizing function with dead basic block --> - [x] #54704 <!-- LazyString in reinterpretarray error messages --> - [x] #54718 <!-- fix prepend StackOverflow issue --> - [x] #54674 <!-- Reimplement dummy pkg prompt as standard prompt --> - [x] #54737 <!-- LazyString in interpolated error messages involving types --> - [x] #54642 <!-- Document GenericMemory and AtomicMemory --> - [x] #54713 <!-- make: use `readelf` for LLVM symbol version detection --> - [x] #54760 <!-- REPL: improve prompt! async function handler --> - [x] #54606 <!-- fix double-counting and non-deterministic results in `summarysize` --> - [x] #54759 <!-- REPL: Fully populate the dummy Pkg prompt --> - [x] #54702 <!-- lowering: Recognize argument destructuring inside macro hygiene --> - [x] #54678 <!-- Don't let setglobal! implicitly create bindings --> - [x] #54730 <!-- Fix uuidkey of exts in fast path of `require_stdlib` --> - [x] #54765 <!-- Handle no-postdominator case in finalizer pass --> - [x] #54591 <!-- Don't expose guard pages to malloc_stack API consumers --> - [x] #54755 <!-- [TOML] remove Dates hack, replace with explicit usage --> - [x] #54721 <!-- add try/catch around scheduler to reset sleep state --> - [x] #54631 <!-- Avoid concatenating LazyString in setindex! for triangular matrices --> - [x] #54322 <!-- effects: add new `@consistent_overlay` macro --> - [x] #54785 - [x] #54865 - [x] #54815 - [x] #54795 - [x] #54779 - [x] #54837 Contains multiple commits, manual intervention needed: - [ ] #52694 <!-- Reinstate similar for AbstractQ for backward compatibility --> - [ ] #54649 <!-- Less restrictive copyto! signature for triangular matrices --> Non-merged PRs with backport label: - [ ] #54779 <!-- make recommendation clearer on manifest version mismatch --> - [ ] #54739 <!-- finish implementation of upgradable stdlibs --> - [ ] #54738 <!-- serialization: fix relocatability bug --> - [ ] #54574 <!-- Make ScopedValues public --> - [ ] #54457 <!-- Make `String(::Memory)` copy --> - [ ] #53957 <!-- tweak how filtering is done for what packages should be precompiled --> - [ ] #53452 <!-- RFC: allow Tuple{Union{}}, returning Union{} --> - [ ] #53286 <!-- Raise an error when using `include_dependency` with non-existent file or directory --> - [ ] #51479 <!-- prevent code loading from lookin in the versioned environment when building Julia -->
KristofferC
removed
the
backport 1.11
Following the discussion in #54642 Implemented: - [x] `modifyindex_atomic!`, `swapindex_atomic!`, `replaceindex_atomic!` for `GenericMemory` - [x] `getindex_atomic`, `setindex_atomic!`, `setindexonce_atomic!` for `GenericMemory` - [x] add support for references in `@atomic` macros - [x] add support for vararg indices in `@atomic` macros - [x] tests - [x] update docstrings with example usage - ~[ ] update Atomics section of the manual (?)~ - [x] news @oscardssmith @vtjnash # New `@atomic` transformations implemented here: ```julia 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)) ``` --------- Co-authored-by: Oscar Smith <oscardssmith@gmail.com>
Closes #53854. After talking with @vtjnash, we are ready to commit to the
GenericMemoryinterface. Sorry @nsajko that this took me so long to get around to.