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

[PERF]: Optimized docker image #1613

Merged
merged 2 commits into from
Jan 22, 2024

Conversation

tazarov
Copy link
Contributor

@tazarov tazarov commented Jan 5, 2024

Description of changes

Summarize the changes made by this PR.

  • Improvements & Bug fixes
    • Removed the re-download of hnsw in docker entrypoint
      • Removed build dependencies in final image to reduce size (by about 100M)
      • Added flag for rebuilding hnsw lib from source
      • Added docs how to build in image with hardware-optimized hnsw

Test plan

How are these changes tested?

  • Tests pass locally with pytest for python, yarn test for js

Documentation Changes

Example of building AVX-optimized image added under examples/.

- Removed the re-download of hnsw
- Removed build dependencies in final image to reduce size
- Added flag for rebuilding hnsw lib from source
- Added docs how to build in image with hardware-optimized hnsw
Copy link

github-actions bot commented Jan 5, 2024

Reviewer Checklist

Please leverage this checklist to ensure your code review is thorough before approving

Testing, Bugs, Errors, Logs, Documentation

  • Can you think of any use case in which the code does not behave as intended? Have they been tested?
  • Can you think of any inputs or external events that could break the code? Is user input validated and safe? Have they been tested?
  • If appropriate, are there adequate property based tests?
  • If appropriate, are there adequate unit tests?
  • Should any logging, debugging, tracing information be added or removed?
  • Are error messages user-friendly?
  • Have all documentation changes needed been made?
  • Have all non-obvious changes been commented?

System Compatibility

  • Are there any potential impacts on other parts of the system or backward compatibility?
  • Does this change intersect with any items on our roadmap, and if so, is there a plan for fitting them together?

Quality

  • Is this code of a unexpectedly high quality (Readability, Modularity, Intuitiveness)

@HammadB
Copy link
Collaborator

HammadB commented Jan 8, 2024

this is great, thanks

Copy link
Contributor

@beggers beggers left a comment

Choose a reason for hiding this comment

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

Could we make it default to true to keep current behavior in the default case? Seems like a very reasonable arg to expose.

@tazarov
Copy link
Contributor Author

tazarov commented Jan 10, 2024

Could we make it default to true to keep current behavior in the default case? Seems like a very reasonable arg to expose.

The only reason I didn't make it default is that our images are built on GH runners, which are running on Intel Xeon, which does have AVX enabled, so the rebuilt lib inside the docker will not work on non-AVX machines. May be I'm missing something though

@tazarov tazarov marked this pull request as ready for review January 18, 2024 06:13
@tazarov
Copy link
Contributor Author

tazarov commented Jan 18, 2024

@beggers, can you take another look here and get this merged?

@beggers
Copy link
Contributor

beggers commented Jan 18, 2024

This looks good to me but @HammadB is more knowledgable about these dark arts than I.

@HammadB HammadB merged commit d22b87b into chroma-core:main Jan 22, 2024
97 checks passed
@tazarov tazarov mentioned this pull request Jan 22, 2024
2 tasks
tazarov added a commit to amikos-tech/chroma-core that referenced this pull request Feb 10, 2024
## Description of changes

*Summarize the changes made by this PR.*
 - Improvements & Bug fixes
	 - Removed the re-download of hnsw in docker entrypoint
- Removed build dependencies in final image to reduce size (by about
100M)
         - Added flag for rebuilding hnsw lib from source
         - Added docs how to build in image with hardware-optimized hnsw

## Test plan
*How are these changes tested?*

- [x] Tests pass locally with `pytest` for python, `yarn test` for js

## Documentation Changes
Example of building AVX-optimized image added under `examples/`.
@tazarov tazarov deleted the feature/optimized-docker-builds branch July 26, 2024 11:33
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.

3 participants