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

[Embedding] Fix coredump when destruct FAE object and enable auto selection of ValuePtr. #114

Merged
merged 1 commit into from
Mar 18, 2022

Conversation

lixy9474
Copy link
Collaborator

No description provided.

@@ -281,7 +281,7 @@ class EmbeddingVar : public ResourceBase {
EmbeddingFilter<K, V, EmbeddingVar<K, V>>* filter_;

~EmbeddingVar() override {
if (emb_config_.is_primary()) {
if (emb_config_.is_primary() && emb_config_.primary_emb_index == 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

加个comment

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

switch (sc_.type) {
case StorageType::DRAM:
VLOG(1) << "StorageManager::DRAM: " << name_;
kvs_.push_back(std::make_pair(new LocklessHashMap<K, V>(), ev_allocator()));
kvs_.push_back(std::make_pair(new LocklessHashMap<K, V>(), cpu_allocator()));
Copy link
Collaborator

Choose a reason for hiding this comment

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

改回 ev allocator ,有问题的ev_allocator 已经revert了

StorageConfig(StorageType t,
const std::string& p,
int64 s) : type(t), path(p), size(s) {}
int64 s,
const std::string layout) : type(t), path(p), size(s) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

在这个类是Embedding整体的配置

以后是要对每一级的layout 配置,存在多级存储下面有不同的layout的情况

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

这个在HBM多级的时候会加上修改

case LayoutType::NORMAL_FIX:
new_value_ptr_fn_ = [] (Allocator* alloc, size_t size) { return new NormalContiguousValuePtr<V>(alloc, size); };
break;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

加 default label

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

bool IsMultiLevel() {
return is_multi_level_;
}

void DebugString() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

加了新的配置,修改DebugString 方法
改成TF的标准样式,
std::string DebugString() const {
return strings::StrCat(....................)

if (max_element_size != 0 && false_positive_probability != -1.0){
kHashFunc = calc_num_hash_func(false_positive_probability);
num_counter = calc_num_counter(max_element_size, false_positive_probability);
} else {
kHashFunc = 0;
num_counter = 0;
}
if (layout_type == LayoutType::NORMAL_FIX) {
if (layout == "normal_fix") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个名字,再refine一下

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

@lixy9474 lixy9474 force-pushed the fix_fae_destruct branch 2 times, most recently from 48c7a22 to 879c92f Compare March 15, 2022 02:06
self._layout = "normal"
self._layout = "normal_fix"
self._layout = "light"

Copy link
Collaborator

Choose a reason for hiding this comment

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

这里需要梳理一下各个功能与layout的选择

@candyzone
Copy link
Collaborator

rerun test @bot

@liutongxuan liutongxuan merged commit 728046a into DeepRec-AI:main Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants