ENH: use c11 atomics for numpy#30489
Conversation
a57b9e4 to
8584677
Compare
numpynumpy
ngoldbaum
left a comment
There was a problem hiding this comment.
Nice simplification. I agree that relaxed loads should be fine - in the worst case there will be some blocking or duplication of work in code paths for cache misses.
Just to be extra sure, I wrote a little test script which doesn't produce any race reports on a TSan-instrumented build of 3.14:
import numpy as np
from numpy.testing._private.utils import run_threaded
dt = np.dtype(('i4', [('a', '<i2'), ('b', '<i2')]))
def worker(b):
b.wait()
hash(dt)
run_threaded(worker, 500, pass_barrier=True)
I'd like someone else who knows numpy's build infrastructure on Windows to weigh in about the new experimental build flag.
rgommers
left a comment
There was a problem hiding this comment.
Nice, this is worth trying indeed.
The /experimental:c11atomics was added in VS 2022 version 17.5 Preview 2, which I believe corresponds to toolchain version 19.35. Right now the minimum we support is 19.20, so before merging we should bump that version number on line 28 of the top-level meson.build file to 19.35. That way we'll get a clear error message if the installed compiler is too old.
Once that is changed, this LGTM I think.
I have updated the msvc version. |
|
It occurs to me: let's add a release note for the MSVC minimum version bump too. |
Where do I need to add it? I am not familar with how numpy handles release notes. |
|
See the |
|
Thanks, I added it to the release notes |
|
CI failures are due to transient network issues during the docs build and are unrelated. Let’s merge this and we’ll see if anyone running against our |
Experiment: use c11 atomics for numpy instead of hand-written atomic functions.