-
Notifications
You must be signed in to change notification settings - Fork 84
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cqy123456 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@cqy123456 Please associate the related issue to the body of your Pull Request. (eg. “issue: #”) |
241ec61
to
0cd8b37
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Let's don't put this many codes with different purposes in a single PR.
- Do we cleaned all OMP in DiskANN? I guess we still have some in Load path right?
- Any performance change for this PR?
// query at a time. More threads will result in higher aggregate query throughput, but will also use more IOs/second | ||
// across the system, which may lead to higher per-query latency. So find the balance depending on the maximum | ||
// number of IOPs supported by the SSD. | ||
CFG_INT num_threads; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will existed code crash due to num_threads
turning to invalid?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put this key into ext_legal_json_keys, and it still legal.
@@ -493,7 +504,6 @@ int generate_pq_data_from_pivots(const std::string data_file, | |||
LOG_KNOWHERE_DEBUG_ << "Processing points [" << start_id << ", " << end_id | |||
<< ").."; | |||
|
|||
#pragma omp parallel for firstprivate(block_data_tmp) schedule(static, 8192) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it OK here to not replace omp with any pool?
0cd8b37
to
244aaa7
Compare
244aaa7
to
306d9c7
Compare
306d9c7
to
9e63eee
Compare
Signed-off-by: cqy123456 <[email protected]>
9e63eee
to
84390fc
Compare
/lgtm |
issue: #782
Comparison between omp and thread pool on building diskann as follow:
dataset: sift1m128d
threads number: 12