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
Add compile option WITH_PMEM and runtime config enable_pmem #329
Conversation
Signed-off-by: Yu, Peng <peng.yu@intel.com>
There will be another pull request to rename variables and update license headers. |
The original pull request to be split: #327 |
engine/allocator.hpp
Outdated
uint64_t max_offset_; | ||
|
||
bool thread_local_counter_enabled_{}; | ||
std::vector<std::int64_t> thread_allocated_sizes_; |
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.
False sharing, potential bottleneck.
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'd like to use std::int64_t*
in next commit.
engine/dl_list.cpp
Outdated
@@ -143,16 +143,20 @@ bool DLList::Replace(DLRecord* old_record, DLRecord* new_record, | |||
old_record->PersistPrevNT(new_record_offset); | |||
} else { | |||
new_record->prev = prev_offset; | |||
#ifdef KVDK_WITH_PMEM |
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.
Maybe we should redefine pmem_persist()/pmem_memcpy()/pmem_memmove() as macros for compiling without pmem so that we can reduce macros?
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.
Good suggestion. I find 7
locations where checking KVDK_WITH_PMEM
could be removed with this improvement.
Signed-off-by: Yu, Peng <peng.yu@intel.com>
Signed-off-by: Yu, Peng <peng.yu@intel.com>
What problem does this PR solve?
Problem Summary: Introducing optional volatile KV storage on DRAM
What is changed and how it works?
What's Changed:
WITH_PMEM
, which isON
by defaultenable_pmem
inKVDK::Configs
, which istrue
by default-DWITH_PMEM=OFF
to remove dependency tolibpmem
-DWITH_PMEM=ON
, settingConfigs.enable_pmem
=false
can make KVDK store data on DRAMCheck List
Tests
Side effects