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

implement of the engine and rewrite factory #209

Merged
merged 1 commit into from
Dec 21, 2024
Merged

Conversation

LHT129
Copy link
Collaborator

@LHT129 LHT129 commented Dec 12, 2024

issue: #203

  • allocator shared_ptr hold in Index and Dataset
  • the allocator ptr always point to SafeAllocator
  • allocator ptr is used inner index
  • only Engine/Resource can create Allocator Object

@LHT129 LHT129 added the kind/feature New feature or request label Dec 12, 2024
@LHT129 LHT129 self-assigned this Dec 12, 2024
@LHT129 LHT129 force-pushed the engine branch 6 times, most recently from d42b997 to e150eaf Compare December 12, 2024 10:17
@LHT129 LHT129 force-pushed the engine branch 5 times, most recently from e9758e4 to 822e77c Compare December 12, 2024 11:31
@LHT129 LHT129 marked this pull request as ready for review December 13, 2024 05:58
@LHT129 LHT129 changed the title [WIP] implement of the engine and rewrite factory implement of the engine and rewrite factory Dec 13, 2024
src/algorithm/hgraph.cpp Show resolved Hide resolved
src/factory/factory.cpp Show resolved Hide resolved
@@ -36,7 +36,7 @@ class MemoryIO : public BasicIO<MemoryIO> {
}

MemoryIO(const JsonType& io_param, const IndexCommonParam& common_param)
: allocator_(common_param.allocator_) {
: allocator_(common_param.allocator_.get()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the MemoryIO's lifetime is shorter than Index, so only raw pointer is enough

src/safe_allocator.h Show resolved Hide resolved
@LHT129 LHT129 force-pushed the engine branch 2 times, most recently from 58c1a5e to 2ed3275 Compare December 17, 2024 08:35
@LHT129 LHT129 force-pushed the engine branch 3 times, most recently from 3006049 to b1445c9 Compare December 18, 2024 10:27

tl::expected<std::shared_ptr<Index>, Error>
CreateIndex(const std::string& name, const std::string& parameters);

// tl::expected<DatasetPtr, Error>
// CreateDataset();

Copy link
Collaborator

Choose a reason for hiding this comment

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

delete unnecessary modifications

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator

@wxyucs wxyucs left a comment

Choose a reason for hiding this comment

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

@LHT129 we need to add an example for engine.

@LHT129 LHT129 force-pushed the engine branch 2 times, most recently from 0aad381 to 48c7871 Compare December 20, 2024 03:48
- allocator shared_ptr hold in Index and Dataset
- the allocator ptr always point to SafeAllocator
- allocator ptr is used inner index
- only Engine/Resource can create Allocator Object

Signed-off-by: LHT129 <[email protected]>
#include "index/hnsw_zparameters.h"
#include "index/index_common_param.h"
#include "vsag/vsag.h"
#include "ThreadPool.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

why include it

Copy link
Collaborator

@inabao inabao left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@wxyucs wxyucs left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Collaborator

@jiaweizone jiaweizone left a comment

Choose a reason for hiding this comment

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

LGTM

src/algorithm/hgraph.cpp Show resolved Hide resolved
@LHT129 LHT129 merged commit 2b3510c into antgroup:main Dec 21, 2024
7 checks passed
@LHT129 LHT129 deleted the engine branch December 21, 2024 04:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants