stdlib audit: atomic refcounting for Bigarray proxy#11177
stdlib audit: atomic refcounting for Bigarray proxy#11177Octachron merged 6 commits intoocaml:trunkfrom
Conversation
This test is here for the review process, but it is too slow and useless to keep in the testsuite after review.
runtime/bigarray.c
Outdated
| free(b->data); | ||
| } else { | ||
| if (-- b->proxy->refcount == 0) { | ||
| if (caml_atomic_refcount_decr(&b->proxy->refcount) == 0) { |
There was a problem hiding this comment.
I thought refcount_decr returns the value before decrement. In this case it should be == 1, not == 0.
There was a problem hiding this comment.
Indeed, that was a stupid mistake. Fixed.
xavierleroy
left a comment
There was a problem hiding this comment.
This looks good to me w.r.t. bigarray handling.
For channels, I'm not convinced the refcount should be atomic, once proper locking is implemented (see #11171).
In particular, caml_atomic_refcount_get is a code smell: it makes no sense to check the value of a reference count if it can be modified concurrently; the only case it makes sense is if locking prevents changes, in which case it doesn't need to be atomic.
|
I propose to remove the channel change for this PR for now, in order to move forward with the bigarray fix. |
xavierleroy
left a comment
There was a problem hiding this comment.
Looks good to me, thanks!
Add a caml/atomic_refcount.h header for atomic refcounting and use the new operations when counting the number of live bigarray proxies.
Add a caml/atomic_refcount.h header for atomic refcounting and use the new operations when counting the number of live bigarray proxies.
Add a caml/atomic_refcount.h header for atomic refcounting and use the new operations when counting the number of live bigarray proxies.
Add a caml/atomic_refcount.h header for atomic refcounting and use the new operations when counting the number of live bigarray proxies.
Add a caml/atomic_refcount.h header for atomic refcounting and use the new operations when counting the number of live bigarray proxies.
This PR implements naive atomic refcounting for bigarray proxy.
The atomic refcounting functions are factorized between
io.candbigarray.cin order to facilitate the switch to a better implementation.The second commit contains a test that fails currently on trunk but which is too slow to keep beyond the lifetime of this PR.
(See #10832 for more information.)