rust-prometheus icon indicating copy to clipboard operation
rust-prometheus copied to clipboard

Trying to use AtomicU64 and AtomicI64 which prevents to build for PPC 32 bits

Open n4ss opened this issue 5 years ago • 14 comments

Describe the bug Trying to have rust-prometheus as a dependency on powerpc-unknown-linux-gnu 32-bits, which yields the following: unresolved imports std::sync::atomic::AtomicI64, std::sync::atomic::AtomicU64

These are not defined on this architecture.

Expected behavior A clear and concise description of what you expected to happen.

System information

  • CPU architecture: PPC32
  • Distribution and kernel version: Linux

n4ss avatar Apr 09 '20 19:04 n4ss

(more specifically, rust-prometheus 0.8.0 , and the failing file is prometheus/src/atomic64.rs)

n4ss avatar Apr 09 '20 19:04 n4ss

Thanks for the report!

AtomicI64 and AtomicU64 are newly added to provide IntGauge / IntCounter.

Looks like we may need to put them behind an arch gate, not providing such optimization counters (and thus prevent importing unsupported atomics).

@overvenus

breezewish avatar Apr 16 '20 04:04 breezewish

We could do something similar to https://github.com/spacejam/sled/pull/1076 falling back to a Mutex<u64> on platforms without AtomicU64 along the lines of what https://github.com/bltavares/atomic-shim does.

What do you think @n4ss @breeswish?

mxinden avatar May 22 '20 18:05 mxinden

@mxinden I prefer to drop them, since IntGauge/Counter is only a performance optimization and is not a standard structure. If we use Mutex<u64> to implement them, this "optimization" becomes even slower than the normal Gauge/Counter.. How do you think?

breezewish avatar Jun 02 '20 05:06 breezewish

I am not sure I understand the issue at hand correctly. As far as I can tell both AtomicU64 and AtomicI64 are not supported on 32-bit PowerPC. Thereby this is not about the IntGauge/Counter optimization but rather about the fundamental building block of the entire library.

One could either:

  • Build ones own AtomicU64 via a Mutex<u64>

  • or use AtomicU32 (but somehow handle overflows in a user friendly manner).


PowerPC and MIPS platforms with 32-bit pointers do not have AtomicU64 or AtomicI64 types.

https://doc.rust-lang.org/std/sync/atomic/

mxinden avatar Jun 12 '20 09:06 mxinden

I think offering AtomicU32 (or AtomicUsize) is probably the best thing to do on platforms which don't have 64 bit integer atomic types. The Prometheus application itself (see links in #364) correctly handles wrapping counters, as long as they don't wrap more than once between samples.

Given the typical use case of Prometheus (data is collected at or below 1 minute intervals), and the relative speed of 32 bit PPC and MIPS platforms, counters which increment faster than 70 million times per second (would wrap a 32 bit unsigned value in ~ 1 minute) are probably reasonably exceptional. Also this restriction could be clearly documented (e.g. so the crate user could work-around e.g. by defining the counter as KiB instead of bytes).

FWIW, my use case for this is to implement a lightweight Prometheus exporter to run on OpenWRT routers and wifi access points. Rust is well suited for this because:

  • Rust supports all of the OpenWRT platforms - these often include 32bit PowerPC and 32bit MIPS - I have a 32 bit MIPS (with no FPU) about a meter away from me, and a 32 bit PowerPC in the next room.

  • It's lighter on memory and CPU than the alternative prometheus exporter library languages (nearly all of OpenWRT is implemented in C, Bourne Shell, or Lua).

n.b. with reference to the above, forcing the use of floating point counters on these platforms may be unacceptably expensive, since like the above example, it's quite common for such embedded processors to have no floating point unit - so that all floating point operations are implemented with integer arithmetic (either by the compiler, or by the operating system kernel).

tim-seoss avatar Nov 06 '20 10:11 tim-seoss

See also: https://github.com/rust-lang/rust/issues/32976

tim-seoss avatar Nov 06 '20 12:11 tim-seoss

Thanks for the detailed suggestion @tim-seoss.

I think offering AtomicU32 (or AtomicUsize) is probably the best thing to do on platforms which don't have 64 bit integer atomic types.

Having a platform specific AtomicU32 on platforms that don't support 64bit atomics, which documents its overflow behavior, sounds good to me. Unless there are any objections by other maintainers, would you, @tim-seoss, like to make a proposal via a pull request?

mxinden avatar Nov 12 '20 09:11 mxinden

OK, I'll see what I can do.

tim-seoss avatar Nov 12 '20 12:11 tim-seoss

I believe having same issue, while building for mips:

$ cargo build --target mips-unknown-linux-musl --release
   <...>
   Compiling prometheus v0.13.0
error[E0432]: unresolved imports `std::sync::atomic::AtomicI64`, `std::sync::atomic::AtomicU64`
 --> /home/mk/.cargo/registry/src/github.com-1ecc6299db9ec823/prometheus-0.13.0/src/atomic64.rs:7:25
  |
7 | use std::sync::atomic::{AtomicI64 as StdAtomicI64, AtomicU64 as StdAtomicU64, Ordering};
  |                         ^^^^^^^^^^^^^^^^^^^^^^^^^  ^^^^^^^^^^^^^^^^^^^^^^^^^ no `AtomicU64` in `sync::atomic`
  |                         |
  |                         no `AtomicI64` in `sync::atomic`
  |
help: a similar name exists in the module
  |
7 | use std::sync::atomic::{AtomicI8 as StdAtomicI64, AtomicU64 as StdAtomicU64, Ordering};
  |                         ~~~~~~~~
help: a similar name exists in the module
  |
7 | use std::sync::atomic::{AtomicI64 as StdAtomicI64, AtomicU8 as StdAtomicU64, Ordering};
  |                                                    ~~~~~~~~

error[E0432]: unresolved import `std::sync::atomic::AtomicU64`
 --> /home/mk/.cargo/registry/src/github.com-1ecc6299db9ec823/prometheus-0.13.0/src/histogram.rs:8:14
  |
8 |     atomic::{AtomicU64 as StdAtomicU64, Ordering},
  |              ---------^^^^^^^^^^^^^^^^
  |              |
  |              no `AtomicU64` in `sync::atomic`
  |              help: a similar name exists in the module: `AtomicU8`

error[E0432]: unresolved import `std::sync::atomic::AtomicU64`
 --> /home/mk/.cargo/registry/src/github.com-1ecc6299db9ec823/prometheus-0.13.0/src/timer.rs:1:37
  |
1 | use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
  |                                     ^^^^^^^^^
  |                                     |
  |                                     no `AtomicU64` in `sync::atomic`
  |                                     help: a similar name exists in the module: `AtomicU8`

For more information about this error, try `rustc --explain E0432`.
error: could not compile `prometheus` due to 3 previous errors
warning: build failed, waiting for other jobs to finish...
    Building [======================>  ] 106/111: tokio


error: build failed

karnauskas avatar Nov 22 '21 10:11 karnauskas

I have the same issue on mips-unknown-linux-musl @karnauskas

eutampieri avatar May 26 '22 12:05 eutampieri

same here, any progress on this ?

nanne007 avatar Aug 18 '22 03:08 nanne007

ping

yinheli avatar Aug 31 '22 13:08 yinheli

I'm not likely to get time to work on this in the remainder of this year, however the change looks pretty small if someone else wants to tackle it.

tim-seoss avatar Sep 25 '22 08:09 tim-seoss