Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AVX support #590

Closed
petrkozorezov opened this issue May 8, 2023 · 6 comments · Fixed by #710
Closed

AVX support #590

petrkozorezov opened this issue May 8, 2023 · 6 comments · Fixed by #710

Comments

@petrkozorezov
Copy link

petrkozorezov commented May 8, 2023

By default, polars python library builds with AVX/AVX2/... support and there is a separate version for legacy processors.
I checked explorer code and didn't found any mentions of AVX. So a tried to build explorer with RUSTFLAGS from polars and saw 4% performance increasing in my own code.
So maybe follow polars python library way: prebuild two versions of explorer (with (by default) and without AVX support) and switch between them with a flag (or smth like that)?

How I built explorer with AVX support:

  Mix.install(
    [
      {:rustler, ">= 0.0.0", optional: true},
      {:explorer, "0.5.4"},
    ],
    system_env: [
      EXPLORER_BUILD: "1",
      RUSTFLAGS: "-C target-feature=+fxsr,+sse,+sse2,+sse3,+ssse3,+sse4.1,+sse4.2,+popcnt,+avx,+fma",
    ]
  )

UPD: Enabling AVX2 adds me another 1% of performance.

@josevalim
Copy link
Member

Sounds good to me! We may need some changes in Rustler precompiled so we include this as a flag too.

@philss
Copy link
Member

philss commented Sep 23, 2023

Hey @petrkozorezov , there is a PR adding this feature: #710

Would you mind to share your opinions? Do you think it covers what you proposed?

I couldn't cover all the targets that run in x86_64 because some of them didn't compile with the flag (Apple one), or I couldn't easily detect if the target was running with the features needed enabled. But for the later, a compile env (or a env var) can be used to turn on the "legacy mode". WDYT?

@petrkozorezov
Copy link
Author

petrkozorezov commented Sep 26, 2023

Hey @petrkozorezov , there is a PR adding this feature: #710

Would you mind to share your opinions? Do you think it covers what you proposed?

I couldn't cover all the targets that run in x86_64 because some of them didn't compile with the flag (Apple one), or I couldn't easily detect if the target was running with the features needed enabled. But for the later, a compile env (or a env var) can be used to turn on the "legacy mode". WDYT?

Looks great! But it's a little bit unclear (without going deeper into the code) how to use legacy mode build (and that it's even exists). Maybe add a few words about it to README?

@philss
Copy link
Member

philss commented Sep 28, 2023

@petrkozorezov Thanks! I added a note to the README.md.

I'm going to hold the PR until we decide to launch a new version, since this is going to be a breaking change due to the dependency on RustlerPrecompiled ~> 0.7.

@josevalim
Copy link
Member

@philss we can merge if you want. tokenizers depends on ~> 0.6, so it should work with ~> 0.7 as well just fine. :)

@philss
Copy link
Member

philss commented Sep 28, 2023

@josevalim done! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants