crossbeam-rs / crossbeam

Tools for concurrent programming in Rust
Apache License 2.0
7.41k stars 468 forks source link

Use AtomicU64 for epochs in 32-bit architectures #433

Open jeehoonkang opened 5 years ago

jeehoonkang commented 5 years ago

@stjepang said:

Just so we don't forget, in all the other places where we have atomic timestamps, we should use AtomicU64 (I believe we can't use wide sequences like in this PR, unfortunately):

  • epoch representation
  • head/tail index in deque, channel, and queues

But that can come later in another PR. It will also require Rust 1.34 minimum.

jeehoonkang commented 4 years ago

Since MSRV = 1.36 now, we can use AtomicU64 now: https://doc.rust-lang.org/std/sync/atomic/struct.AtomicU64.html

taiki-e commented 3 years ago

Note that some 32-bit architectures do not support AtomicU64: https://github.com/tokio-rs/tokio/pull/1421#issuecomment-527911780

jeehoonkang commented 3 years ago

Very sad. Do you think it's worth it to use AtomicU64 for those platforms that support it? My opinion is yes, because, e.g., major 32-bit architectures support AtomicU64.

taiki-e commented 3 years ago

I think it's good to use it when AtomicU64 is available, but we have to decide what to do if AtomicU64 is not available. We can probably use AtomicCell<u64> for this, but performance may be degraded on platforms that AtomicU64 is not available.

pkubaj commented 3 years ago

Note that there are already issues with building this crate on 32-bit powerpc:

error[E0432]: unresolved imports `core::sync::atomic::AtomicI64`, `core::sync::atomic::AtomicU64`
  --> /wrkdirs/usr/ports/benchmarks/inferno/work/inferno-0.10.6/cargo-crates/crossbeam-utils-0.8.5/src/lib.rs:79:49
   |
79 |             pub(crate) use core::sync::atomic::{AtomicI64, AtomicU64};
   |                                                 ^^^^^^^^^  ^^^^^^^^^ no `AtomicU64` in `sync::atomic`
   |                                                 |
   |                                                 no `AtomicI64` in `sync::atomic`
   |
help: a similar name exists in the module
   |
79 |             pub(crate) use core::sync::atomic::{AtomicI8, AtomicU64};
   |                                                 ^^^^^^^^
help: a similar name exists in the module
   |
79 |             pub(crate) use core::sync::atomic::{AtomicI64, AtomicU8};
   |                                                            ^^^^^^^^

error[E0412]: cannot find type `AtomicU64` in module `core::sync::atomic`
    --> /wrkdirs/usr/ports/benchmarks/inferno/work/inferno-0.10.6/cargo-crates/crossbeam-utils-0.8.5/src/atomic/consume.rs:78:14
     |
78   |   impl_atomic!(AtomicU64, u64);
     |                ^^^^^^^^^ help: a struct with a similar name exists: `AtomicU16`

error[E0412]: cannot find type `AtomicI64` in module `core::sync::atomic`
    --> /wrkdirs/usr/ports/benchmarks/inferno/work/inferno-0.10.6/cargo-crates/crossbeam-utils-0.8.5/src/atomic/consume.rs:80:14
     |
80   |   impl_atomic!(AtomicI64, i64);
     |                ^^^^^^^^^ help: a struct with a similar name exists: `AtomicI16`

error: aborting due to 3 previous errors

Some errors have detailed explanations: E0412, E0432.
For more information about an error, try `rustc --explain E0412`.
error: could not compile `crossbeam-utils`
taiki-e commented 3 years ago

@pkubaj Hmm... that's odd. https://github.com/crossbeam-rs/crossbeam/pull/698 was supposed to have taken care of those problems.

Which build system did you use? And did you receive any warnings during the build?

pkubaj commented 3 years ago

Thanks for the link. I'm building on FreeBSD (so using powerpc-unknown-freebsd target, which is not yet upstreamed). This is why it fails - powerpc-unknown-freebsd is not in the pre-generated no_atomic.rs. After adding it, build passes.

For now, I patched it in our ports tree. I guess we must send our powerpc port to upstream.