crossbeam icon indicating copy to clipboard operation
crossbeam copied to clipboard

How can I enable `--cfg=crossbeam_loom` in a dependency?

Open davidbarsky opened this issue 9 months ago • 5 comments

Hello! I'm trying to enable Loom in https://github.com/salsa-rs/salsa/. Since Salsa uses crossbeam (namely, SegQueue and AtomicCell) to write some fiddly, unsafe code, I wanted to also enable crossbeam's crossbeam_loom feature flag. Unfortunately, I've been struggling to enable it via RUSTFLAGS. For instance, a env RUSTFLAGS="--cfg=crossbeam_loom" cargo check doesn't seem to enable correctly enable crossbeam_util's Loom flag, failing with with errors along the lines of:

error[E0433]: failed to resolve: use of undeclared crate or module `loom`
  --> /Users/dbarsky/.cargo/registry/src/index.crates.io-6f17d22bba15001f/crossbeam-utils-0.8.21/src/lib.rs:49:24
   |
49 |         pub(crate) use loom::hint::spin_loop;
   |                        ^^^^ use of undeclared crate or module `loom`

error[E0433]: failed to resolve: use of undeclared crate or module `loom`
  --> /Users/dbarsky/.cargo/registry/src/index.crates.io-6f17d22bba15001f/crossbeam-utils-0.8.21/src/lib.rs:53:28
   |
53 |             pub(crate) use loom::sync::atomic::{
   |                            ^^^^ use of undeclared crate or module `loom`

error[E0433]: failed to resolve: use of undeclared crate or module `loom`
  --> /Users/dbarsky/.cargo/registry/src/index.crates.io-6f17d22bba15001f/crossbeam-utils-0.8.21/src/lib.rs:63:28
   |
63 |             pub(crate) use loom::sync::atomic::fence as compiler_fence;
   |                            ^^^^ use of undeclared crate or module `loom`

error[E0433]: failed to resolve: use of undeclared crate or module `loom`
  --> /Users/dbarsky/.cargo/registry/src/index.crates.io-6f17d22bba15001f/crossbeam-utils-0.8.21/src/lib.rs:65:24
   |
65 |         pub(crate) use loom::sync::{Arc, Condvar, Mutex};
   |                        ^^^^ use of undeclared crate or module `loom`

How can I enable crossbeam's Loom flag? Is it... even possible?

davidbarsky avatar Jan 30 '25 19:01 davidbarsky

Loom is cfg-ed optional dependency, so you have to enable both crossbeam_loom cfg and loom feature.

https://github.com/crossbeam-rs/crossbeam/blob/75d24e0a4ca4733c09b4bde0c137f4120cccaf04/crossbeam-utils/Cargo.toml#L34-L39

This is a bit odd, but unfortunately necessary to address the problem that the way cargo and crates.io handle cfg-ed dependencies is not very good, confuses users, and some people complain when new dependency appears in Cargo.lock (even if it never actually builds). (e.g., https://github.com/crossbeam-rs/crossbeam/pull/487#issuecomment-783365041, https://github.com/crossbeam-rs/crossbeam/issues/665, https://github.com/tokio-rs/bytes/issues/411, https://github.com/tokio-rs/bytes/issues/400, etc.) See also https://github.com/crossbeam-rs/crossbeam/pull/666.

taiki-e avatar Jan 30 '25 19:01 taiki-e

Ah, thanks for your answer! The following:

[target.'cfg(crossbeam_loom)'.dependencies]
crossbeam-utils = { version = "0.8", features = ["loom"] }
crossbeam-epoch = { version = "0.9", features = ["loom"] }

...with a RUSTFLAGS="--cfg=crossbeam_loom" cargo check mostly does the trick. The only remaining issue is AtomicCell not being available under Loom. I can open an new issue for this, but would you accept a PR that partially backs out https://github.com/crossbeam-rs/crossbeam/pull/787 and documents that the size might not be the same under Loom, or would that break some non-trivial things in crossbeam?

davidbarsky avatar Jan 30 '25 20:01 davidbarsky

The issue is that they do not work because they have different structures to begin with. The implementation before https://github.com/crossbeam-rs/crossbeam/pull/787 is completely broken.

AFAIK, what could be implemented is a fallback implementation, such as the one referenced by the review comment in that PR, or one that just uses Mutex internally. (Or change something on the Loom side.)

So, for now, I would suggest using Mutex or RwLock instead of AtomicCell when using Loom.

taiki-e avatar Jan 30 '25 21:01 taiki-e

The implementation before https://github.com/crossbeam-rs/crossbeam/pull/787 is completely broken.

Ah, I didn't realize that was the case. Good to know.

AFAIK, what could be implemented is a fallback implementation, such as the one referenced by the review comment in that PR, or one that just uses Mutex internally. (Or change something on the Loom side.) So, for now, I would suggest using Mutex or RwLock instead of AtomicCell when using Loom.

I can use a lock in the short-term when consuming crossbeam. If you'd like, I'd also be happy to have crossbeam itself fallback to Mutex internally, as SeqQueue uses AtomicCell and I think it'd be nice to extend Loom testing to crossbeam's queues.

davidbarsky avatar Jan 31 '25 19:01 davidbarsky

as SeqQueue uses AtomicCell

If you are talking about crossbeam_queue::SegQueue, crossbeam_queue doesn't use AtomicCell at all. (In crossbeam, only crossbeam_channel::tick uses AtomicCell)

The reason why crossbeam_queue does not currently support Loom is for a completely different reason. When I tried it before, unbounded queue panicked with odd errors like https://github.com/tokio-rs/loom/issues/283.

taiki-e avatar Jan 31 '25 19:01 taiki-e