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

Add optimized implementations #6

Open
James-E-A opened this issue Jan 15, 2024 · 5 comments
Open

Add optimized implementations #6

James-E-A opened this issue Jan 15, 2024 · 5 comments
Labels
enhancement New feature or request

Comments

@James-E-A
Copy link
Owner

Fix whatever cursed compile errors are blocking that.

@James-E-A
Copy link
Owner Author

Also, this will require adding some logic to choose the best implementation.

I think import-time is the way to go for this, though there seem to be a few ways to approach it:

  • Benchmark all the implementations when the module is imported, like [can't remember which package, but it's a popularish one] does
  • Depend on a 3rd-party Python library such as py-cpuinfo
  • Roll our own CPU-detection code
  • Steal vendor the code from an existing 3rd-party Python library

To avoid the pain trilemma of "reinvent a wheel, poorly; or torture users with Hellfire by pinning a dependency version; or risk breakage by using a non-pinned dependency", I think that vendoring might be a good choice.

@James-E-A James-E-A added the enhancement New feature or request label Jan 19, 2024
@James-E-A
Copy link
Owner Author

Fix whatever cursed compile errors are blocking that.

according to the OpenQuantumSafe Wiki, those implementations aren't for Windows anyway: https://openquantumsafe.org/liboqs/algorithms/kem/classic_mceliece.html#classic-mceliece-6960119f-implementation-characteristics

god knows if they're correct. might be worth asking upstream.

@James-E-A
Copy link
Owner Author

As an aside, it looks like py-cpuinfo has retained the same API from version 0.2.3 (8 years ago) – version 9.0.0 (current), though AVX2 didn't arrive till later (version 4.0.0).

So at least

risk breakage by using a non-pinned dependency

isn't so great.

from cpuinfo import get_cpu_info  # pip install "py-cpuinfo >= 4"

HAS_AVX2 = 'avx2' in get_cpu_info()['flags']

@James-E-A
Copy link
Owner Author

PQClean/PQClean#532 (comment)

(Emphasis added:)

[SPHINCS+ AVX2] implementations support Windows … [however,] most AVX2 implementations rely on assembly code, and assembler syntax is not portable: gas syntax is not understood by Microsoft's assembler.

As always, we're open to contributions to improve this situation for algorithms that are currently not supporting Windows.

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

No branches or pull requests

1 participant