Repository: duckdb/duckdb
File: src/include/duckdb/common/lru_cache.hpp (C++)

Code before:
class SharedLruCache {
public:
	using key_type = Key;
	using mapped_type = shared_ptr<Val>;
	using hasher = KeyHash;
	using key_equal = KeyEqual;

	// @param max_memory_p: Maximum total memory (in bytes) of entries. 0 means unlimited.
	explicit SharedLruCache(idx_t max_memory_p) : max_memory(max_memory_p), current_memory(0) {
	}

	// Disable copy and move
	SharedLruCache(const SharedLruCache &) = delete;
	SharedLruCache &operator=(const SharedLruCache &) = delete;

	~SharedLruCache() = default;

	// Insert `value` with key `key` and explicit memory size. This will replace any previous entry with the same key.
	void Put(Key key, shared_ptr<Val> value, unique_ptr<BufferPoolReservation> reservation) {
		if (reservation == nullptr) {
			throw InvalidInputException("Reservation cannot be null when emplace into LRU!");
		}

		// Remove existing entry if present
		const idx_t memory_size = reservation->size;
		auto existing_it = entry_map.find(key);
		if (existing_it != entry_map.end()) {
			DeleteImpl(existing_it);
		}

		// Evict entries if needed to make room
		if (max_memory > 0 && memory_size > 0) {
			EvictIfNeeded(memory_size);
		}

		// Add new entry
		lru_list.emplace_front(key);
		Entry new_entry;
		new_entry.value = std::move(value);
		new_entry.lru_iterator = lru_list.begin();
		new_entry.reservation = std::move(reservation);

		entry_map[std::move(key)] = std::move(new_entry);
		current_memory += memory_size;
	}

	// Delete the entry with key `key`.
	// Return whether the requested `key` is found in the cache.
	bool Delete(const Key &key) {
		auto it = entry_map.find(key);
		if (it == entry_map.end()) {

Review comment (performance):
Just curious, why do we prefer variadic template to construct the payload, instead of a plain value? Would appreciate it if we could document it. :)

Just my personal preference, I usually implement an API like `Put(key, val, payload)`, the reason being: this class is not thread-safe for now, having such interface makes things easier if we want to make it thread-safe
```cpp
class ThreadSafeLru {
  template <typename... Types>
  void Put(key, val, Types.. args) {
      // payload construction could expensive, and should be placed outside of critical section.
      auto payload = make_uniq<Payload>(std::forward<Types>(constructor_args)...);

     const lock_guard(mutex);
     // Delegate to thread-safe version.
   }
};
```

Code after:
	}
};

// A LRU cache implementation, whose value could be accessed in a shared manner with shared pointer.
// Notice, it's not thread-safe.
template <typename Key, typename Val, typename Payload = DefaultPayload, typename KeyHash = std::hash<Key>,
          typename KeyEqual = std::equal_to<Key>>
class SharedLruCache {
public:
	using key_type = Key;
	using mapped_type = shared_ptr<Val>;
	using hasher = KeyHash;
	using key_equal = KeyEqual;

	// @param max_total_weight_p: Maximum total weight (in relevant unit) of entries. 0 means unlimited.
	explicit SharedLruCache(idx_t max_total_weight_p) : max_total_weight(max_total_weight_p), current_total_weight(0) {
	}

	// Disable copy and move
	SharedLruCache(const SharedLruCache &) = delete;
	SharedLruCache &operator=(const SharedLruCache &) = delete;

	~SharedLruCache() = default;

	// Insert `value` with key `key` and explicit memory size. This will replace any previous entry with the same key.
	template <typename... Types>
	void Put(Key key, shared_ptr<Val> value, Types... constructor_args) {
		// Remove existing entry if present
		auto existing_it = entry_map.find(key);
		if (existing_it != entry_map.end()) {
			DeleteImpl(existing_it);
		}

		auto payload = make_uniq<Payload>(std::forward<Types>(constructor_args)...);
		auto payload_weight = payload->GetWeight();

		// Evict entries if needed to make room
		if (max_total_weight > 0 && payload_weight > 0) {
			EvictIfNeeded(payload_weight);
		}

		// Add new entry
		lru_list.emplace_front(key);
		Entry new_entry;
		new_entry.value = std::move(value);
		new_entry.lru_iterator = lru_list.begin();
		new_entry.payload = std::move(payload);
		new_entry.payload_weight = payload_weight;

		entry_map[std::move(key)] = std::move(new_entry);
		current_total_weight += payload_weight;