vpetrigo / caches Goto Github PK

Hi,

It works for iOS and Android?

thanks.

I tried this library, it's easy to use, high quality and thread safe. I am going to use it in my projects.
According to the situation I used, I want to exchange my thoughts about below 2 features.

Sometimes we need to do something based the value we get from cache as line 9 and line 13 showed. I noticed that if the key is not in the cache, the library will throw an exception. In some way, to handle the exception makes uncomfortable :).

May you add below two features? Many Thanks.

1. add a overload function for Get() that return an boolean value which mark the value is found or not in the cache and the the value of key as the second parameter.
2. Change the access permission for Erase() to public and make it thread safe. If the value is satisfied with a condition, We may erase the key directly.

1. constexpr std::size_t CACHE_SIZE = 256;
2. lru_cache_t<std::string, int> cache(CACHE_SIZE);
3. 
4. std::string key{"Hello"};
5. cache.Put("Hello", 1);
6. cache.Put("world", 2);
7. 
8. int value = 0;
9. auto ret = cache.Get(key,value);
10. if(ret && value >= 2){
11. cout << "key is grater than 2.\n";
12. ** cache.Erase(key); **
13. } else {
14. cache.Put(key,++value);
15. }

Below is a similar code accroding to your implementation. FYI.

  const bool Get(const Key& key,Value& value) const {
    operation_guard{safe_op};
    auto elem_it = FindElem(key);

    if (elem_it == cache_items_map.end()) {
        return false;
    }
    cache_policy.Touch(key);

    value = elem_it->second;
    return true;
  }

By the way, have you had a benchmark? Thank you for your library again.

cache.hpp seems like the current locking through using operation_guard{safe_op}; (without naming a local variable) is not locking at all. Changing it to for example: "operation_guard lk{safe_op};" fixes it for me.

设置每个item的时间，如果时间到了，cache中的的item失效丢弃，即超时过期清除

The line:
https://github.com/vpetrigo/caches/blob/6fd6d318b74ea4400fab3142fb61862d8a8252e3/include/cache_policy.hpp#LL67C5-L67C49

produces an unused parameter warning when compiled with -Wunused-parameter.

Suggest changing this line to the following to disable the warning only on this method signature.

void Touch(__attribute__((unused)) const Key &key) noexcept override

Online module have another map instead of unorder_map, could this caches module support thirdparty map implement?

The current FIFO cache policy always erases from the back of the list. This is correct when the removal is due to cache eviction, but not correct when removing a specific key (e.g. in the middle of the FIFO list).

// fifo_cache_tests.cpp:83 TEST(FIFOCache, Remove_Test) {

std::string removed_key;
int removed_value = -1;
auto callback = [&](const std::string& key, const int& value){
    removed_key = key;
    removed_value = value;
};

constexpr std::size_t TEST_SIZE = 10;
fifo_cache_t <std::string, std::size_t> fc(TEST_SIZE, {}, callback);

for (std::size_t i = 0; i < TEST_SIZE; ++i) {
    fc.Put(std::to_string(i), i);
}

// test removing key in the middle of FIFO list
const int specific_key = 5;
EXPECT_TRUE(fc.Remove(std::to_string(specific_key)));
EXPECT_TRUE(removed_key == std::to_string(specific_key));  // These _say_ they removed key == "5", 
EXPECT_TRUE(removed_value == specific_key);                // but by debugging, I see that key == "0" is removed from 'fifo_queue'

// ...

how to add this cache library using cmake so that i can use like this
#include <lru_cache_policy.hpp>
#include <cache.hpp>

name space should be added to this line
using lru_cache_t = typename caches::fixed_sized_cache<Key, Value, LRUCachePolicy<Key>>;

to be
using lru_cache_t = typename caches::fixed_sized_cache<Key, Value, caches::LRUCachePolicy<Key>>;

have a good day

@vpetrigo Thanks a lot for releasing a truly useful library for many applications! Currently, there is no multithreading support in your code. How would you recommend to add basic thread coherency into your implementation?

code is here: https://github.com/vpetrigo/caches/blob/master/include/cache_policy.hpp#L43

According to http://en.cppreference.com/w/cpp/container/unordered_set, unordered_set does not have that function. It is unordered :P

Change the access permission for Erase() to public and make it thread safe. If the value is satisfied with a condition, We may erase the key directly.

Hi,

I think there is a conception design issue in the thread safe part.
Even internal code methods are protected by mutex, the API doesn't look to be threadsafe:
Indeed, TryGet return an iterator that can be invalidate by concurrent Put.
The same with Get that return a const reference, you can't guaranty the reference is still valid outside the Get method.

Regards,
Laurent