MIPS support

[bltavares]

Hi there,

I've recently tried to compile a project that depends on sled to OpenWRT mips device using cross.

 cross build --target mips-unknown-linux-musl

This led me to type errors similar to the ones found on async-std for 32bit devices.

I've tried to locally change the types from AtomicU64 to AtomicUsize as they did there, and there were many more errors to fix...

I'm curious if there is interest on having the project working on MIPS devices, and it I could give it a try to get it compiling. Cheers.

[bltavares]

This is certainly wrong in subtle ways but it still compiles on x64, but it made some good progress compiling for mips.

I was able to compile for it, but I have no idea if it is working

Surprisingly, only two tests failed, and the same on both x64 and mips:

failures:
    serialization::qc::data
    serialization::qc::node

I would love to hear If you have any recommendations on the way to go, or even a guideline on how to cast things. I've tried following the compiler errors as casting to make it fix hehe

[divergentdave]

I think the underlying issue isn't the processor word size, but the absence of the atomic memory instructions. (sled works on i686 as-is) The docs say AtomicU64 isn't supported on 32-bit MIPS or PowerPC. https://doc.rust-lang.org/std/sync/atomic/index.html#portability AtomicU64 is baked in pretty deeply to the architecture of sled, you may notice that the IoBuf header actually holds multiple bit-packed fields, for example.

[bltavares]

Thanks for replying so fast! As you can notice, I don't know much about MIPS or Atomic Instructions.

Given that only two tests failed on the serialization part, do you think it is even worth to try to make it pass, given you mentioned that AtomicU64 is core to how sled works?

I know MIPS is quite niche, and I have only a passer-by interest at this moment, with at most a hobbyist application in the far future (eg: embed a db for wifi configs). I would be ok to close the issue if it is too far fetch to support it.

[spacejam]

does Mutex exist for your MIPS target? If so, we may have nice luck by lust conditionally compiling an AtomicU64 for MIPS that is the underlying 64-bit item wrapped in a Mutex, and just ignores the Ordering argument that is passed to its methods. It may also be nice for doing things like asserting that (certain, all?) CAS operations never fail when testing sled in single-threaded mode.

[bltavares]

Yes, Mutex seems to be supported.

Running the following shows that it works:

fn main() {
    let mutex = std::sync::Mutex::new(std::u64::MAX);
    let result = mutex.lock();
    println!("Hello, world! Mutex: {:?}", result);
}

root@ArcherC7:~# ./test-mips
Hello, world! Mutex: Ok(18446744073709551615)

Given what you mentioned, I thought of adding a wrapping type on sled that is either an AtomicU64 or a Mutex and provide the same API used on the Atomic struct, with a conditional compilation of what Default does.

#[cfg(not(target_arch="mips"))]
impl Default for AtomicOrMutex {
  fn default() -> Self {
    AtomicOrMutext::Atomic(AtomizeU64::default())
  }
}

#[cfg(target_arch="mips")]
impl Default for AtomicOrMutex {
  fn default() -> Self {
    AtomicOrMutext::Mutex(Mutex::<u64>::default())
  }
}

I could give this a try if that is sound and aligned with the project goals.

[divergentdave]

Rather than making an Enum with two variants, I think it would make for a better dev experience to put use std::sync::atomic::AtomicU64 behind a conditional compilation condition, and then name the mutex-based shim type AtomicU64, and put it behind the opposite conditional compilation condition. (besides, enum AtomicOrMutex { Atomic(AtomicU64), .. } would not build on the 32-bit MIPS target) Something like this:

#[cfg(not(any(target_arch = "mips", target_arch = "powerpc")))]
use std::sync::atomic::AtomicU64;

#[cfg(any(target_arch = "mips", target_arch = "powerpc"))]
mod atomic_shim;

#[cfg(any(target_arch = "mips", target_arch = "powerpc"))]
use atomic_shim::AtomicU64;

[bltavares]

Would it be worth to use atomic64 which seems to provide this feature?

It looks a bit outdated, but a useful shim overall to be extracted to a crate.

[divergentdave]

That crate is definitely on the right track for this, but, as you mention, it's out of date, plus it's using a nightly-only feature from back before const fn had stabilized. Additionally, we may want to use crossbeam's Mutex in lieu of std::sync::Mutex, for its performance.

[bltavares]

I've extracted the Atomic shims into a separate crate, as I thought it would be useful for other projects.

I would be ok if you would rather vendor/copy the code into the code base instead of adding an external library.

I spend the day creating a QEMU MIPS VM to run all tests, as cross-compiling the tests had a few problems when spawning a sub-process, something about executing the wrong architecture. Most of the tests passes, but there was a few issues with some tests on the tests folder (cgroup memory limits and slow execution time for measuring wall time).

I'll be opening a PR with the changes for review, but I would be more than fine to copy the file for the project :)

[fogti]

crossbeam does provide an Atomic shim, too: AtomicCell.

[bltavares]

Huum, that could be a good use instead of building my own shim replacement. Thanks for sharing :)

[bltavares]

Now that #1076 has landed this issue could be closed :)

Thanks for the support folks