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 a FAISS_OPT_LEVEL=avx512_spr build mode. #2404

Merged
merged 10 commits into from
Jan 24, 2025

Conversation

mulugetam
Copy link
Contributor

Description

FAISS recently added a new build mode, avx512_spr, which enables the use of advanced AVX512 instructions available since Intel® Sapphire Rapids. These instructions include avx512_fp16, avx512_bf16, avx512_vpopcntdq, and many others. Some optimizations leveraging these instructions, such as speeding up Hamming distance evaluation for binary vectors, have already been implemented and merged. Additional optimizations are also in the pipeline. This PR enables K-NN to utilize this new avx512_spr build mode.

The code changes are implemented such that, by default, if the underlying system is an Intel® Sapphire Rapids or a newer-generation processor, avx512_spr binaries are generated. Otherwise, depending on the features available in the system and consistent with the existing implementation, avx512, avx2, or generic binaries may be generated.

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff.
  • Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

mulugetam and others added 3 commits January 17, 2025 07:20
  - avx512_spr enables advanced avx512 instructions available since Intel(R) Sapphire Rapids.

Signed-off-by: Mulugeta Mammo <[email protected]>
Signed-off-by: Assane Diop <[email protected]>
# else if (system_supports_avx512) generate_avx512_binaries()
# else if (system_supports_ avx2) generate_avx2_binaries()
# else() generate_generic_binaries()
./gradlew build -Davx2.enabled=true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove this line

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean lines 294-298?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, line 298

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

scripts/build.sh Outdated Show resolved Hide resolved
scripts/build.sh Outdated Show resolved Hide resolved
@naveentatikonda
Copy link
Member

@mulugetam Thanks for making these changes. Can you fix the failing CI and add CHANGELOG

@naveentatikonda naveentatikonda added backport 2.x Features Introduces a new unit of functionality that satisfies a requirement labels Jan 17, 2025
@mulugetam
Copy link
Contributor Author

@naveentatikonda Looks like the CI is using gcc 10.5.0 (we need 12.1+). Will proceed with fixing the other issues (which are mainly formatting).

mulugetam and others added 4 commits January 17, 2025 16:03
Co-authored-by: Naveen Tatikonda <[email protected]>
Signed-off-by: mulugetam <[email protected]>
Co-authored-by: Naveen Tatikonda <[email protected]>
Signed-off-by: mulugetam <[email protected]>
Signed-off-by: Mulugeta Mammo <[email protected]>
@naveentatikonda
Copy link
Member

@mulugetam also pls fix the failing unit test related to avx512_spr. Thanks!

scripts/build.sh Outdated Show resolved Hide resolved
@mulerm
Copy link

mulerm commented Jan 21, 2025

Thanks! Could you re-run CI?

Signed-off-by: Naveen Tatikonda <[email protected]>
@naveentatikonda
Copy link
Member

I tried to cherry-pick these changes and built them on Peter's custom image with GCC 12.4 without any compile-time or runtime issues when tested with some sample curl commands.

@naveentatikonda
Copy link
Member

BWC framework is flaky and the failing BWC tests not related to these changes

@peterzhuamazon
Copy link
Member

Pending opensearch-project/opensearch-build#5241 to be merged and images to be pushed before new tests would take effect.

@naveentatikonda
Copy link
Member

@mulugetam can you pls share some performance benchmarking results when tested from opensearch comparing avx512_spr results with avx512 and avx2 atleast for a couple of datasets ?

@mulugetam
Copy link
Contributor Author

@naveentatikonda We're currently working on that and will share it when its ready. For a quick check, you may run hamming bench with avx512_spr.

@peterzhuamazon
Copy link
Member

New image is online we can retry the test on this PR.

Thanks.

@naveentatikonda naveentatikonda merged commit 90fdad4 into opensearch-project:main Jan 24, 2025
33 of 34 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jan 24, 2025
* Add avx512_spr option to FAISS_OPT_LEVEL.

  - avx512_spr enables advanced avx512 instructions available since Intel(R) Sapphire Rapids.

Signed-off-by: Mulugeta Mammo <[email protected]>

* Fix documentation.

Signed-off-by: Assane Diop <[email protected]>

* Update scripts/build.sh

Co-authored-by: Naveen Tatikonda <[email protected]>
Signed-off-by: mulugetam <[email protected]>

* Update scripts/build.sh

Co-authored-by: Naveen Tatikonda <[email protected]>
Signed-off-by: mulugetam <[email protected]>

* Fix formatting issues.

Signed-off-by: Mulugeta Mammo <[email protected]>

* Update CHANGELOG.md for avx512_spr build mode.

Signed-off-by: Mulugeta Mammo <[email protected]>

* Fix bugs in build options and avx512_spr flag testing.

Signed-off-by: Mulugeta Mammo <[email protected]>

* Upgrade gcc version to 12.4.

Signed-off-by: Mulugeta Mammo <[email protected]>

---------

Signed-off-by: Mulugeta Mammo <[email protected]>
Signed-off-by: Assane Diop <[email protected]>
Signed-off-by: mulugetam <[email protected]>
Signed-off-by: Naveen Tatikonda <[email protected]>
Co-authored-by: Assane Diop <[email protected]>
Co-authored-by: Naveen Tatikonda <[email protected]>
(cherry picked from commit 90fdad4)
naveentatikonda added a commit that referenced this pull request Jan 27, 2025
* Add a FAISS_OPT_LEVEL=avx512_spr build mode. (#2404)

* Add avx512_spr option to FAISS_OPT_LEVEL.

  - avx512_spr enables advanced avx512 instructions available since Intel(R) Sapphire Rapids.

Signed-off-by: Mulugeta Mammo <[email protected]>

* Fix documentation.

Signed-off-by: Assane Diop <[email protected]>

* Update scripts/build.sh

Co-authored-by: Naveen Tatikonda <[email protected]>
Signed-off-by: mulugetam <[email protected]>

* Update scripts/build.sh

Co-authored-by: Naveen Tatikonda <[email protected]>
Signed-off-by: mulugetam <[email protected]>

* Fix formatting issues.

Signed-off-by: Mulugeta Mammo <[email protected]>

* Update CHANGELOG.md for avx512_spr build mode.

Signed-off-by: Mulugeta Mammo <[email protected]>

* Fix bugs in build options and avx512_spr flag testing.

Signed-off-by: Mulugeta Mammo <[email protected]>

* Upgrade gcc version to 12.4.

Signed-off-by: Mulugeta Mammo <[email protected]>

---------

Signed-off-by: Mulugeta Mammo <[email protected]>
Signed-off-by: Assane Diop <[email protected]>
Signed-off-by: mulugetam <[email protected]>
Signed-off-by: Naveen Tatikonda <[email protected]>
Co-authored-by: Assane Diop <[email protected]>
Co-authored-by: Naveen Tatikonda <[email protected]>
(cherry picked from commit 90fdad4)

* Fix failing rolling upgrade workflow

Signed-off-by: Naveen Tatikonda <[email protected]>

* Update init-faiss.cmake

Signed-off-by: Naveen Tatikonda <[email protected]>

---------

Signed-off-by: Naveen Tatikonda <[email protected]>
Co-authored-by: mulugetam <[email protected]>
Co-authored-by: Naveen Tatikonda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Features Introduces a new unit of functionality that satisfies a requirement v2.19.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants