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 ConcurrentLRUCache #1268
Add ConcurrentLRUCache #1268
Conversation
std::lock_guard<std::mutex> guard(lock_); | ||
auto v = lru_->get(key); | ||
if (v == boost::none) { | ||
return Status::Error(); | ||
} | ||
return std::move(v).value(); |
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.
Ask a question,do we need insert it to cache immediate If this key is not exist in cache ?
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.
Agreed, maybe we need another method putIfAbsent
Unit testing passed. |
LGTM |
Jenkins go |
Unit testing passed. |
void clear() { | ||
for (auto i = 0; i < bucketsNum_; i++) { | ||
buckets_[i].clear(); | ||
} |
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 need buckets_.clear() at here?
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.
When we call clear, what we want is to clear the content in cache, not destroy the buckets.
@@ -0,0 +1,79 @@ | |||
/* Copyright (c) 2018 vesoft inc. All rights reserved. |
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.
2019 ?
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 catch
std::lock_guard<std::mutex> guard(lock_); | ||
auto v = lru_->get(key); | ||
if (v == boost::none) { | ||
return Status::Error(); |
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.
Status::Error(folly::stringPrintf("%s not found", key))
. would be better ?
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.
It will hurt the performance because miss cache is normal in practice.
Unit testing passed. |
Unit testing passed. |
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.
LGTM, great job!
Unit testing passed. |
* Add ConcurrentLRUCache * Address sky's comments * update mod * Add evict for specified key * Remove dependency on time obj
No description provided.