Remove multiversion dependency

[tustvold]

Which issue does this PR close?

Closes #.

Rationale for this change

We only use this in a single location, and in general recommend people compile with the appropriate target-cpu, as such I think we can remove this without any major issue.

Eliminating a dependency is always a good thing, especially as this results in additional codegen, and reduces our vulnerability to upstream shenanigans.

What changes are included in this PR?

Are there any user-facing changes?

[alamb]

https://github.com/calebzulawski/target-features appears to be a one hit wonder crate (basically one version, no obvious plan for maintenance)

https://github.com/calebzulawski/target-features/commits/master

I am all for removing this dependency 👍

[alamb]

It would be nice to ping whoever initially added the multiversion dependency as a courtesy -- I didn't do enough code archeology to really figure it out

[ursabot]

Benchmark runs are scheduled for baseline = 61a77a5 and contender = ec2fd47. ec2fd47 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped ⚠️ Benchmarking of arrow-rs-commits is not supported on ec2-t3-xlarge-us-east-2] ec2-t3-xlarge-us-east-2
[Skipped ⚠️ Benchmarking of arrow-rs-commits is not supported on test-mac-arm] test-mac-arm
[Skipped ⚠️ Benchmarking of arrow-rs-commits is not supported on ursa-i9-9960x] ursa-i9-9960x
[Skipped ⚠️ Benchmarking of arrow-rs-commits is not supported on ursa-thinkcentre-m75q] ursa-thinkcentre-m75q
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

[alamb]

Related upstream ticket: calebzulawski/target-features#1

[Dandandan]

It would be nice to ping whoever initially added the multiversion dependency as a courtesy -- I didn't do enough code archeology to really figure it out

I believe I did. The goal was to be able to use more SIMD for some kernels.
The benefit I see of using multiversion would be compiling for a somewhat older CPU, while benefiting from runtime feature detection for newer CPUs (and not having to build an unportable binary. Although the use now is very limited...

[calebzulawski]

That's precisely the use-case for multiversion. I understand if multiversion isn't appropriate for this project, but I don't think it's reasonable to expect users to use target-cpu beyond local testing or targeting a particular embedded system.

https://github.com/calebzulawski/target-features appears to be a one hit wonder crate (basically one version, no obvious plan for maintenance)

https://github.com/calebzulawski/target-features/commits/master

I am all for removing this dependency 👍

target-features was created mostly to address issues discovered while creating the std::simd module that didn't quite make it to inclusion in the standard library. I decided to keep it separate from multiversion because some users may want to access only the target-features hardware database without all of the multiversioning apparatus.

[alamb]

Thank you for the response @calebzulawski

[tustvold]

I don't think it's reasonable to expect users to use target-cpu beyond local testing or targeting a particular embedded system

I don't disagree that it is definitely not ideal, but it also is not really feasible for us to generate multiple versions of all our kernels, as we already have severe issues with the amount of codegen and the corresponding build times.

Correct me if I am mistaken, but if a user enabled AVX using target-cpu the produced binary will support 95% of the CPUs in the latest Steam Hardware Survey. A more aggressive target-cpu=haswell will still support almost all CPUs from this decade. I'm inclined to think most people can safely make this trade-off?

[alamb]

I'm inclined to think most people can safely make this trade-off?

I personally think it is more beneficial to choose the appropriate target architecture higher up at the application level rather than in a lower level library.

For example, I would rather compile several versions of our top level influxdb_iox binary for various different architectures / instruction sets and pick between those binaries in a launcher rather than have the choice done inside of the low level arrow library

I would prefer the top level approach because more of my application could take advantage of the target cpu's instructions (via llvm's support for auto vectorization, for example), rather than just arrow.

[calebzulawski]

I don't think it's reasonable to expect users to use target-cpu beyond local testing or targeting a particular embedded system

I don't disagree that it is definitely not ideal, but it also is not really feasible for us to generate multiple versions of all our kernels, as we already have severe issues with the amount of codegen and the corresponding build times.

Correct me if I am mistaken, but if a user enabled AVX using target-cpu the produced binary will support 95% of the CPUs in the latest Steam Hardware Survey. A more aggressive target-cpu=haswell will still support almost all CPUs from this decade. I'm inclined to think most people can safely make this trade-off?

In this case, yes, AVX is commonly available. When AVX512 stabilizes in Rust, however, you would likely not want to build an entire application that requires it. If the amount of codegen is a concern then removing multiversioning can definitely help--you could also consider using a cargo feature to control whether or not multiversioning is present, and leave it up to the users. Another thing to consider is that particular software distributions (e.g. Linux) probably have particular CPUs they target, and won't be able to take advantage of target-cpu.

I'm inclined to think most people can safely make this trade-off?

I personally think it is more beneficial to choose the appropriate target architecture higher up at the application level rather than in a lower level library.

For example, I would rather compile several versions of our top level influxdb_iox binary for various different architectures / instruction sets and pick between those binaries in a launcher rather than have the choice done inside of the low level arrow library

I would prefer the top level approach because more of my application could take advantage of the target cpu's instructions (via llvm's support for auto vectorization, for example), rather than just arrow.

That is another possibility, one that's used by Intel's MKL for example. I'm not sure it's the best approach, however:

• If the amount of codegen (or binary size) is a concern, this is the worst option, since all code is duplicated
• Rust still doesn't (and perhaps never will) have a stable ABI
• It's OS specific (dlopen etc) and precludes users from making statically linked executables
• The vast majority of code, from what I've seen, does not benefit from vectorization. MKL is a special case, where nearly every function it provides is vectorized.

With all of this in mind, I don't have any opinion on what is the best option for arrow, but it might be helpful if I summarized all of this in multiversion's documentation.

[alamb]

It's OS specific (dlopen etc) and precludes users from making statically linked executables

I guess what I had in mind was literally create builds of the influxdb_iox binary for each architecture (well really probably "AVX512" and "SSE2", and maybe a third one in the middle).

As you note Rust doesn't have a stable ABI (and therefore will recompile almost everything from source each time anyways)