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]: Improved server-side serialization with orjson + async #1938

Merged
merged 12 commits into from
Apr 4, 2024
4 changes: 3 additions & 1 deletion chromadb/api/segment.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from functools import cached_property

from chromadb.api import ServerAPI
from chromadb.config import DEFAULT_DATABASE, DEFAULT_TENANT, Settings, System
from chromadb.db.system import SysDB
Expand Down Expand Up @@ -789,7 +791,7 @@ def reset(self) -> bool:
def get_settings(self) -> Settings:
return self._settings

@property
@cached_property
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason for this? it adds a small amount of cognitive overhead ("what's a cached property?") for no benefit that i can see?

@override
def max_batch_size(self) -> int:
return self._producer.max_batch_size
Expand Down
8 changes: 7 additions & 1 deletion chromadb/auth/fastapi.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import asyncio

import chromadb
from contextvars import ContextVar
from functools import wraps
import logging
Expand Down Expand Up @@ -173,7 +176,7 @@ def authz_context(
) -> Callable[[Callable[..., Any]], Callable[..., Any]]:
def decorator(f: Callable[..., Any]) -> Callable[..., Any]:
@wraps(f)
def wrapped(*args: Any, **kwargs: Dict[Any, Any]) -> Any:
async def wrapped(*args: Any, **kwargs: Dict[Any, Any]) -> Any:
_dynamic_kwargs = {
"api": args[0]._api,
"function": f,
Expand Down Expand Up @@ -213,6 +216,7 @@ def wrapped(*args: Any, **kwargs: Dict[Any, Any]) -> Any:
)

if _provider:
# TODO this will block the event loop if it takes too long - refactor for async
a_authz_responses.append(_provider.authorize(_context))
if not any(a_authz_responses):
raise AuthorizationError("Unauthorized")
Expand All @@ -239,6 +243,8 @@ def wrapped(*args: Any, **kwargs: Dict[Any, Any]) -> Any:
):
kwargs["database"].name = desired_database

if asyncio.iscoroutinefunction(f):
return await f(*args, **kwargs)
return f(*args, **kwargs)

return wrapped
Expand Down
2 changes: 2 additions & 0 deletions chromadb/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ def empty_str_to_none(cls, v: str) -> Optional[str]:
return v

chroma_server_nofile: Optional[int] = None
# the number of maximum threads to handle synchronous tasks in the FastAPI server
chroma_server_thread_pool_size: int = 40

chroma_server_auth_provider: Optional[str] = None

Expand Down
1 change: 1 addition & 0 deletions chromadb/db/impl/sqlite.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ def __init__(self, conn_pool: Pool, stack: local):
@override
def __enter__(self) -> base.Cursor:
if len(self._tx_stack.stack) == 0:
self._conn.execute("PRAGMA case_sensitive_like = ON")
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this meant to be included? It seems unrelated to the rest of the changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found this bug while testing the PR, so I added it here. I can split it up in another PR, as it is indeed unrelated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Move to PR or leave it here?

self._conn.execute("BEGIN;")
self._tx_stack.stack.append(self)
return self._conn.cursor() # type: ignore
Expand Down
Loading
Loading