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

fix: perf issue, do not load proto on each search() call #124

Conversation

maksspace
Copy link
Contributor

@maksspace maksspace commented Dec 18, 2022

Issue

I wanted to use this client for a project, but the problem is it's extremely slow because it loads proto on each search() call which takes around 40ms just to load proto.

Here's a benchmark, it's a simple nodejs server that uses milvus client to search:

const http = require('http');
const { query } = require('./db-client');

const server = http.createServer();

function vector(dim) {
  const arr = new Array(dim);
  for (let i = 0; i < dim; i++) {
    arr[i] = Math.random();
  }
  return arr;
}

const vectors = {};

for (let dims = 128; dims <= 2048; dims += 128) {
  vectors[dims] = vector(dims);
}

server.on('request', (request, res) => {
  const dims = +request.url.slice(1);
  
  query('milvus', `random_${dims}`, vectors[dims]).then((d) => {
     return res.end('Ok.');
  }).catch((e) => {
    console.log(e);
    res.writeHead(500);
    return res.end('Bad.')
  });
});

server.listen(8000);

image

Dataset

id as varchar 24, and float vector with 128 dimensions, 500K vectors.

Env:

CPU AMD Ryzen 3900X
32GB ram DDR4
SSD 512GB WD black PCIe 4.0 m2

Results

I think it's speaks for itself :)
image

@sre-ci-robot
Copy link

Welcome @maksspace! It looks like this is your first PR to milvus-io/milvus-sdk-node 🎉

@maksspace maksspace force-pushed the fix-perfomance-issue-do-not-load-proto-on-each-search-call branch from 82b9aea to a273840 Compare December 18, 2022 13:25
@shanghaikid
Copy link
Contributor

/lgtm

@shanghaikid
Copy link
Contributor

/approve

@sre-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: maksspace, shanghaikid

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

@nameczz nameczz merged commit 208d9b2 into milvus-io:main Dec 19, 2022
@shanghaikid
Copy link
Contributor

Thanks a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants