Skip to content

mark dataset test static for test index#164

Merged
LHT129 merged 1 commit into
antgroup:mainfrom
LHT129:feature_test
Nov 28, 2024
Merged

mark dataset test static for test index#164
LHT129 merged 1 commit into
antgroup:mainfrom
LHT129:feature_test

Conversation

@LHT129

@LHT129 LHT129 commented Nov 27, 2024

Copy link
Copy Markdown
Collaborator
  • fix some typo error
  • clean code
  • mark dataset static for test index to speed up functest

@LHT129 LHT129 added the kind/improvement Optimizations, UX polish, or minor improvements 性能优化、体验打磨或细节改良 label Nov 27, 2024
@LHT129
LHT129 requested a review from inabao November 27, 2024 08:37
@LHT129 LHT129 self-assigned this Nov 27, 2024
- fix some typo error
- clean code
- mark dataset static for test index to speed up functest

Signed-off-by: LHT129 <tianlan.lht@antgroup.com>
Comment thread tests/test_index.h

private:
mutable std::unordered_map<std::string, std::shared_ptr<TestDataset>> datasets;
static std::unordered_map<std::string, TestDatasetPtr> datasets;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

will there be concurrency issues here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

no concurrent test for this now

Comment thread tests/test_index.cpp
return result;
}

std::unordered_map<std::string, TestDatasetPtr> TestIndex::datasets = {};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why do we need to make it static?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

dataset may be same for all test

@inabao inabao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@wxyucs wxyucs left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm

@LHT129
LHT129 merged commit 0f59185 into antgroup:main Nov 28, 2024
@LHT129
LHT129 deleted the feature_test branch November 28, 2024 03:49
Comment thread tests/test_index.cpp
auto querys = dataset->query_;
auto query_count = querys->GetNumElements();
auto dim = querys->GetDim();
auto queries = dataset->query_;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we need rename dataset query_ to query ?

Roxanne0321 pushed a commit to Roxanne0321/vsag that referenced this pull request Mar 3, 2025
- fix some typo error
- clean code
- mark dataset static for test index to speed up functest

Signed-off-by: LHT129 <tianlan.lht@antgroup.com>
jiacai2050 pushed a commit to jiacai2050/vsag that referenced this pull request Mar 6, 2025
- fix some typo error
- clean code
- mark dataset static for test index to speed up functest

Signed-off-by: LHT129 <tianlan.lht@antgroup.com>
LHT129 added a commit that referenced this pull request May 11, 2026
- fix some typo error
- clean code
- mark dataset static for test index to speed up functest

Signed-off-by: LHT129 <tianlan.lht@antgroup.com>
Sia-Sheerland pushed a commit to Sia-Sheerland/vsag that referenced this pull request Jun 26, 2026
- fix some typo error
- clean code
- mark dataset static for test index to speed up functest

Signed-off-by: LHT129 <tianlan.lht@antgroup.com>
Signed-off-by: Sia Sheerland <x1075956441x@163.com>

Signed-off-by: Sia Sheerland <x1075956441x@163.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/improvement Optimizations, UX polish, or minor improvements 性能优化、体验打磨或细节改良

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants