u32_backend is slower than u64_backend on armv7 Android

[jrose-signal]

I noticed the upcoming 4.0 release has removed the u32_backend and u64_backend features in favor of checking the target pointer size, which seems very sensible.* We know the u32_backend can be drastically faster than the u64_backend in some configurations:

On my Xperia 10 with SailfishOS (armv7hl user space on aarch64 kernel, yes I know, rustflags = "-C target-feature=+v7,+neon"):

• u32_backend + std: [644.25 us 646.59 us 649.28 us]
• u64_backend + std: [6.4522 ms 6.4685 ms 6.4880 ms]

…but not all:

On a Cortex-M3 (Texas Instruments CC2538):

• u32_backend without alloc: 470507 cycles
• u32_backend with alloc (wut): 878507 cycles
• u64_backend without alloc: 494078 cycles
• u64_backend with alloc: 470502 cycles (the fastest)

(signalapp/libsignal#453 (comment))

Today I finally got around to running some 32-bit microbenchmarks on a new Android phone (with help from a coworker), a Pixel 6a, which I feel should nonetheless still produce interesting results because, well, running armv7 code on an aarch64 device can't magically use 64-bit registers. We consistently found that the u32_backend was 5-15% slower than the u64_backend when compiling for that particular armv7.

EDIT: Later, I tested on a 32-bit Moto X and got approximately the same results.

All of this is differences of at most a tenth of a millisecond on any particular operation (we tested key agreement and signing + verifying), so it's not like it's going to make or break the crate. But it did make me think twice about "u32_backend is for 32-bit CPUs, u64_backend is for 64-bit CPUs". Is that worth restoring feature flags to override the default inference?

* Especially given that the previous config made it easy for a downstream crate to depend on u64_backend implicitly as a default feature, thus removing the chance for the final program to choose the u32_backend instead.

[jrose-signal]

Hypothesis about @rubdos' drastic Xperia results: the compiler is doing a terrible job vectorizing the u64 backend for armv7 (target feature +neon). We don't get that because some 32-bit Android phones really are that low-end.

[tarcieri]

running armv7 code on an aarch64 device can't magically use 64-bit registers. We consistently found that the u32_backend was 5-15% slower than the u64_backend when compiling for that particular armv7.

To be clear: this is a 64-bit device, and you are worried about the performance of running a 32-bit binary on it? If so, why aren't you building a 64-bit binary?

Something else I've generally worried about is whether using the 64-bit backend on a 32-bit target impacts constant-time operation. I haven't done any investigation there, but I think it's worth looking into.

All that said, I think it would be good to have a way to override the selected backend for these particular cases, but using feature flags for this purpose runs afoul of some of the original problems #414 was meant to address, like: what happens if u32_backend and u64_backend are both enabled, or if fiat_backend or simd_backend are enabled as well.

It would probably be good to use a cfg attribute for this purpose, which also solves this problem:

Especially given that the previous config made it easy for a downstream crate to depend on u64_backend implicitly as a default feature, thus removing the chance for the final program to choose the u32_backend instead.

Indeed it's a bit worse than that. If you have any dependencies that explicitly activate different backends it would result in a compile error with pages and pages of type errors.

Using a cfg attribute ensures only the toplevel binary can choose the backend, and using a cfg attribute like e.g. dalek_backend ensures only one backend can be explicitly selected.

[tarcieri]

Also note that if we do switch to cfg attributes, we could potentially ship v4.0 while punting on manual overrides for u32_backend/u64_backend as it would make the backend selection futureproof.

I think it will require some internal refactoring to implement cleanly, moving all of the relevant logic for selecting the backend to a single place so it doesn't need to be duplicated. Right now it's somewhat spread out.

[jrose-signal]

To be clear: this is a 64-bit device, and you are worried about the performance of running a 32-bit binary on it? If so, why aren't you building a 64-bit binary?

Convenience only: Google's Microbenchmark framework doesn't work pre-Android-7, but all our 32-bit test devices are deliberately still on older versions of Android for testing. At some point I'll go find a third-party benchmarking library to test it properly.

I don't love the cfg thing; it does guarantee a single selection, but it loses discoverability, and if you misspell the feature you'll never know. I get that Cargo doesn't provide a perfect answer at the moment, though.

[tarcieri]

Yeah, unfortunately I've found no way to check that a given cfg value is within an allowed set without making it mandatory.

Likewise there's no way to ensure the right name is used for the cfg attribute.

That said, it's the only way to move control to the toplevel binary, rather than transitive dependencies, as well as the only way to ensure a single option is specified versus multiple potential (transitively) activated features like u32_backend and u64_backend.

[burdges]

Just fyi wasm does 64 bit arithmetic but has 32 bit pointers, not sure if you made an exception for that and similar cases. I suppose wasm likely needs other strange things anyways.

[tarcieri]

Yeah, ideally we can factor the backend gating into a single cfg_if! statement so we can include various exemptions like that

[jrose-signal]

Convenience only: Google's Microbenchmark framework doesn't work pre-Android-7, but all our 32-bit test devices are deliberately still on older versions of Android for testing. At some point I'll go find a third-party benchmarking library to test it properly.

Just to circle back: I did a clunky benchmark using System.nanoTime() and a single warmup before many repeated invocations on an old 32-bit Moto X, and the results were reliably about the same as the 64-bit device: the 32-bit backend is 5-15% slower than the 64-bit backend.

…and I only thought later to try with +neon as rubdos did, and it did not make an appreciable difference on this phone. Possibly because both the compiler and RustCrypto have gotten better over the last year.

[jrose-signal]

🤦 Also I typo'd the original report: the difference on both phones is a tenth of a millisecond, which is still pretty small but also 1000 times as big as a tenth of a microsecond.

[tarcieri]

Do you all think it would make sense to enable the 64-bit backend by-default on target_arch = "arm"? (along with WASM)

[rozbb]

So we would have auto-selection by default, with a cfg override? If so, that sounds reasonable to me. How difficult would it be to get an MVP of the cfg flag overrides working so we can include it in the 4.0 breaking changes?

[rubdos]

FWIW, we have a WIP neon-backend version in zkcrypto#19 that I'm cleaning up and making armv7-compatible. Requires rust-lang/stdarch#1363

It's easy enough to rebase to this repository, if that's a wish. I even have a merge with the lizard2 branch lying around (and a merge with the Whisperfish branch, for testing reasons)

I presume most ARMv7 phones will come with NEON, but @jrose-signal will probably know better.