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 chroma to benchmark #205

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

LukasWestholt
Copy link

No description provided.

@LukasWestholt
Copy link
Author

LukasWestholt commented Sep 27, 2024

Hello dear Chroma maintainers,

Are there existing optimisation options and how can they be used (for example CHROMA_WORKERS)?
Thank you.
@tazarov

@LukasWestholt
Copy link
Author

Waiting for chroma-core/chroma#2581

Copy link

@tazarov tazarov left a comment

Choose a reason for hiding this comment

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

A few minor nits and caveats, but generally looks good.

pyproject.toml Outdated Show resolved Hide resolved
engine/clients/chroma/upload.py Show resolved Hide resolved
engine/servers/chroma-single-node/docker-compose.yaml Outdated Show resolved Hide resolved
engine/servers/chroma-single-node/docker-compose.yaml Outdated Show resolved Hide resolved
engine/clients/chroma/__init__.py Show resolved Hide resolved
@LukasWestholt
Copy link
Author

LukasWestholt commented Sep 30, 2024

Is something like this helpful?

@classmethod
def delete_client(cls):
    if cls.client is not None:
        cls.client._admin_client._server._session.close()
        cls.client._server._session.close()

for the problem of chroma-core/chroma#2581.

This is because in the benchmark, search queries are sent in several parallel processes (on the client side) with parallel=100 (multiprocessing). And a new HttpClient instance is created in each of these processes. Does this cause problems? Since Chroma then works serially on the server side, this benchmark probably just doesn't make sense.

This also results in worse times with parallel > 1 than with parallel = 1, I think.

Let me test this right now. Update: Yes, parallel > 1 is slower then parallel = 1, even with this snippet above, so i deleted this part of the benchmark.

…db volume, fix CHROMA_WORKERS, allow more chromadb versions in pyproject.toml
@LukasWestholt LukasWestholt marked this pull request as ready for review October 1, 2024 07:55
@tazarov
Copy link

tazarov commented Oct 3, 2024

Is something like this helpful?

@classmethod
def delete_client(cls):
    if cls.client is not None:
        cls.client._admin_client._server._session.close()
        cls.client._server._session.close()

for the problem of chroma-core/chroma#2581.

This is because in the benchmark, search queries are sent in several parallel processes (on the client side) with parallel=100 (multiprocessing). And a new HttpClient instance is created in each of these processes. Does this cause problems? Since Chroma then works serially on the server side, this benchmark probably just doesn't make sense.

This also results in worse times with parallel > 1 than with parallel = 1, I think.

Let me test this right now. Update: Yes, parallel > 1 is slower then parallel = 1, even with this snippet above, so i deleted this part of the benchmark.

Due to how the WAL is currently implemented, mixing reads and writes will result in lots of blocking, even for reads. Regarding the client.close(), lots of people expressed various degrees of frustration with the lack of it, as depending on your use case, it can exhibit differently—sockets left open, open file handles, failures during shutdown, etc.

I'll try to get the close done shortly.

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.

2 participants