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

pgvector: ensure vector is sent in binary representation #335

Merged
merged 1 commit into from
Jun 28, 2024

Conversation

jkatz
Copy link
Contributor

@jkatz jkatz commented Jun 14, 2024

PostgreSQL supports two methods of passing data from client to server: text and binary. While for many data types the difference may not be noticeable, we can see significant performance impact when converting a vector from binary => text => binary representation. See previous explanation here[1].

While the pgvector loading code accounts for this, the query code did not. This is due to the use of a list[float] type, which the pgvector-python adapter currently doesn't support. However, this adapter does support direct binary transfer if the data is represent as a Numpy array[2]. Testing shows that moving to a direct binary representation does have a significant impact on query results - my tests are showing a 3x impact -- and provides a more accurate representation for how this workload would execute.

[1] erikbern/ann-benchmarks#488
[2] https://github.com/pgvector/pgvector-python?tab=readme-ov-file#psycopg-3

PostgreSQL supports two methods of passing data from client to
server: text and binary. While for many data types the difference
may not be noticeable, we can see significant performance impact
when converting a vector from binary => text => binary representation.
See previous explanation here[1].

While the pgvector loading code accounts for this, the query code
did not. This is due to the use of a list[float] type, which
the pgvector-python adapter currently doesn't support. However,
this adapter does support direct binary transfer if the data
is represent as a Numpy array[2]. Testing shows that moving to
a direct binary representation does have a significant impact on
query results - my tests are showing a 3x impact --  and provides
a more accurate representation for how this workload would execute.

[1] erikbern/ann-benchmarks#488
[2] https://github.com/pgvector/pgvector-python?tab=readme-ov-file#psycopg-3
@jkatz
Copy link
Contributor Author

jkatz commented Jun 15, 2024

/assign @XuanYang-cn

@jkatz
Copy link
Contributor Author

jkatz commented Jun 18, 2024

Updating with some additional testing: the speedup tends to impact queries that have higher throughput, e.g. for HNSW, lower values of hnsw.ef_search. This is due to the high throughput rate and reducing the overhead of converting from binary/text/binary. At higher values of hnsw.ef_search, the impact is less noticeable as more time is spent on the index lookups.

@jkatz
Copy link
Contributor Author

jkatz commented Jun 23, 2024

@XuanYang-cn Does this require additional changes? Thanks!

@sre-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: alwayslove2013, jkatz
To complete the pull request process, please assign xuanyang-cn after the PR has been reviewed.
You can assign the PR to them by writing /assign @xuanyang-cn in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@alwayslove2013 alwayslove2013 merged commit 5265f2f into zilliztech:main Jun 28, 2024
4 checks passed
@alwayslove2013
Copy link
Collaborator

@jkatz Nice! Thank you for bringing this to our attention. We may need to recheck whether this has the same impact on other clients as well.

significant performance impact when converting a vector from binary => text => binary representation.

@jkatz
Copy link
Contributor Author

jkatz commented Jun 28, 2024

@alwayslove2013 Agreed! Thank you for merging.

@jkatz jkatz deleted the pgvector-query-binary branch June 28, 2024 15: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