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 AVX512 support to k-nn FAISS #2069

Closed
wants to merge 76 commits into from

Conversation

akashsha1
Copy link
Contributor

@akashsha1 akashsha1 commented Sep 7, 2024

Description

This change adds support to speed up vector search and indexing in faiss using AVX512 hardware accelerator.

Related Issues

Resolves #2056

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.

build.gradle Outdated Show resolved Hide resolved
build.gradle Outdated Show resolved Hide resolved
jni/cmake/init-faiss.cmake Outdated Show resolved Hide resolved
jni/cmake/init-faiss.cmake Outdated Show resolved Hide resolved
jni/cmake/init-faiss.cmake Outdated Show resolved Hide resolved
jni/cmake/init-faiss.cmake Outdated Show resolved Hide resolved
src/main/java/org/opensearch/knn/jni/FaissService.java Outdated Show resolved Hide resolved
src/main/java/org/opensearch/knn/jni/FaissService.java Outdated Show resolved Hide resolved
src/main/java/org/opensearch/knn/jni/PlatformUtils.java Outdated Show resolved Hide resolved
@naveentatikonda
Copy link
Member

naveentatikonda commented Sep 9, 2024

@akashsha1 Can you also update .github/workflows/CI.yml and .github/workflows/test_security.yml to detect if avx512 is supported or not and pass the flag to gradle.

Also, pls add an entry in CHANGELOG for this PR and sign off the PR to fix DCO check. Thanks!

@naveentatikonda
Copy link
Member

Can you also please update build script to build all 3 versions of faiss lib for the distribution. Pls change this block of code like shown below

if [ "$PLATFORM" != "windows" ] && [ "$ARCHITECTURE" = "x64" ]; then
  echo "Building k-NN library after enabling AVX2"
  # Skip applying patches as patches were applied already from previous :buildJniLib task
  # If we apply patches again, it fails with conflict
  ./gradlew :buildJniLib -Davx2.enabled=true -Dbuild.lib.commit_patches=false -Dbuild.lib.apply_patches=false

  echo "Building k-NN library after enabling AVX512"
  ./gradlew :buildJniLib -Davx512.enabled=true -Dbuild.lib.commit_patches=false -Dbuild.lib.apply_patches=false
fi

@naveentatikonda
Copy link
Member

@akashsha1 You also need to add the avx512 faiss library in plugin-security.policy to provide runtime permissions for loading the library. Pls test end to end after making all these changes. Thanks!

Signed-off-by: Akash Shankaran <[email protected]>
Signed-off-by: Akash Shankaran <[email protected]>
CHANGELOG.md Outdated Show resolved Hide resolved
@naveentatikonda
Copy link
Member

@akashsha1 Looks like some of the commits were not properly signed causing the DCO to fail. Do you want to rebase and squash everything into a single commit and sign that commit ?

image

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

akashsha1 and others added 3 commits September 15, 2024 22:16
Co-authored-by: Naveen Tatikonda <[email protected]>
Signed-off-by: akashsha1 <[email protected]>
Signed-off-by: Akash Shankaran <[email protected]>
Signed-off-by: Akash Shankaran <[email protected]>
naveentatikonda and others added 27 commits September 15, 2024 22:41
* Add HNSW changes to support Faiss byte vector

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

* Address Review Comments

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

* Add IVF changes to support Faiss byte vector

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

* Address Review Comments

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

---------

Signed-off-by: Naveen Tatikonda <[email protected]>
Signed-off-by: Akash Shankaran <[email protected]>
…pensearch-project#2020)

starting 2.17

This removes the feature flag for the same

Signed-off-by: Tejas Shah <[email protected]>
Introduces new params for mapping and training, called compression_level
and mode. These parameters are high level parameters that give the
plugin a hint as to what the user wants to configure their system like
without exposing algorithmic details. This change just adds these
parameters to the plugin as noops. In future change, we will add the
functionality for parameter resolution.

Along with this, I added a class to more easily manage the original
parameters that a user passes. This will help ensure our mapper
maintains good compatibility.

Signed-off-by: John Mazanec <[email protected]>
Signed-off-by: Akash Shankaran <[email protected]>
Signed-off-by: Naveen Tatikonda <[email protected]>
Signed-off-by: Akash Shankaran <[email protected]>
Add quantization state reader and writer

Signed-off-by: Ryan Bogan <[email protected]>
Signed-off-by: Akash Shankaran <[email protected]>
)

* Introduce mode and compression param resolution

Adds mode and compression based parameter resolution. With this, if a
user specifies the mode and/or compression params, we will create a
default configuration with the aim of meeting those hints.

Currently, it does not contain support for overriding any of the
parameters. This will be taken in a future commit.

Signed-off-by: John Mazanec <[email protected]>

* Modify reader changes

Signed-off-by: John Mazanec <[email protected]>

---------

Signed-off-by: John Mazanec <[email protected]>
Signed-off-by: Akash Shankaran <[email protected]>
…pensearch-project#2044)

* Add spaceType as a top level parameter while creating vector field.

Signed-off-by: Navneet Verma <[email protected]>

* fix release notes

Signed-off-by: John Mazanec <[email protected]>

* Remove commented out code

Signed-off-by: John Mazanec <[email protected]>

---------

Signed-off-by: Navneet Verma <[email protected]>
Signed-off-by: John Mazanec <[email protected]>
Co-authored-by: John Mazanec <[email protected]>
Signed-off-by: Akash Shankaran <[email protected]>
…o be from cluster metadata (opensearch-project#2005)

* Add model version to model metadata

Signed-off-by: Ryan Bogan <[email protected]>

* Add model version to model metadata and change model metadata reads to be from cluster metadata

Signed-off-by: Ryan Bogan <[email protected]>

* Add changelog entry

Signed-off-by: Ryan Bogan <[email protected]>

* Set version from config context

Signed-off-by: Ryan Bogan <[email protected]>

* Fix spotless

Signed-off-by: Ryan Bogan <[email protected]>

* Update model index mappings

Signed-off-by: Ryan Bogan <[email protected]>

* Change field mapper to read model version

Signed-off-by: Ryan Bogan <[email protected]>

* Fix tests

Signed-off-by: Ryan Bogan <[email protected]>

* remove println

Signed-off-by: John Mazanec <[email protected]>

---------

Signed-off-by: Ryan Bogan <[email protected]>
Signed-off-by: John Mazanec <[email protected]>
Co-authored-by: John Mazanec <[email protected]>
Signed-off-by: Akash Shankaran <[email protected]>
Signed-off-by: Akash Shankaran <[email protected]>
Signed-off-by: Akash Shankaran <[email protected]>
Co-authored-by: Naveen Tatikonda <[email protected]>
Signed-off-by: akashsha1 <[email protected]>
Signed-off-by: Akash Shankaran <[email protected]>
Signed-off-by: Akash Shankaran <[email protected]>
Signed-off-by: Akash Shankaran <[email protected]>
Co-authored-by: Naveen Tatikonda <[email protected]>
Signed-off-by: akashsha1 <[email protected]>
Signed-off-by: Akash Shankaran <[email protected]>
@akashsha1
Copy link
Contributor Author

Hi @naveentatikonda - please consider this PR: #2110 in favor of the current one. The rebasing due to DCO sign-off got messy, so I created a new PR instead. It has all feedback addressed to this point, except the workflow failures which needs to be resolved.

@akashsha1 akashsha1 closed this Sep 16, 2024
@akashsha1
Copy link
Contributor Author

@akashsha1 We need to add the CPU flag detection logic in these 2 workflows and pass the gradle flags to fix the failing checks https://github.com/opensearch-project/k-NN/blob/main/.github/workflows/backwards_compatibility_tests_workflow.yml https://github.com/opensearch-project/k-NN/blob/main/.github/workflows/test_security.yml

This needs to be addressed in #2110 ..

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 this pull request may close these issues.

[FEATURE] Add support for FAISS AVX512
10 participants