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

LRU: fix eviction order #1092

Merged
merged 5 commits into from
Jun 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions lib/archethic_cache/lru.ex
Original file line number Diff line number Diff line change
Expand Up @@ -128,21 +128,36 @@ defmodule ArchethicCache.LRU do
end

defp evict_until(
state = %{table: table, keys: keys, evict_fn: evict_fn, bytes_used: bytes_used},
state = %{keys: keys},
predicate
) do
# we reverse the keys here so we don't need to reverse them in a loop
state = %{state | keys: Enum.reverse(keys)}
new_state = do_evict_until(state, predicate)
%{new_state | keys: Enum.reverse(new_state.keys)}
end

defp do_evict_until(
state = %{
table: table,
keys: reversed_keys,
evict_fn: evict_fn,
bytes_used: bytes_used
},
predicate
) do
if predicate.(state) do
state
else
case Enum.reverse(keys) do
case reversed_keys do
[] ->
state

[oldest_key | rest] ->
[{_, {size, oldest_value}}] = :ets.take(table, oldest_key)
evict_fn.(oldest_key, oldest_value)

evict_until(
do_evict_until(
%{
state
| bytes_used: bytes_used - size,
Expand Down
41 changes: 26 additions & 15 deletions test/archethic_cache/lru_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,13 @@ defmodule ArchethicCache.LRUTest do
end

test "should evict some cached values when there is not enough space available" do
binary = get_a_binary_of_bytes(200)
binary = :crypto.strong_rand_bytes(200)

{:ok, pid} = LRU.start_link(:my_cache, 500)

LRU.put(:my_cache, :key1, binary)
LRU.put(:my_cache, :key2, binary)
LRU.put(:my_cache, :key3, get_a_binary_of_bytes(400))
LRU.put(:my_cache, :key3, :crypto.strong_rand_bytes(400))

:sys.get_state(pid)

Expand All @@ -58,7 +58,7 @@ defmodule ArchethicCache.LRUTest do
end

test "should evict the LRU" do
binary = get_a_binary_of_bytes(200)
binary = :crypto.strong_rand_bytes(200)

{:ok, pid} = LRU.start_link(:my_cache, 500)

Expand All @@ -76,8 +76,29 @@ defmodule ArchethicCache.LRUTest do
assert nil == LRU.get(:my_cache, :key2)
end

test "should evict the LRU in order" do
binary = :crypto.strong_rand_bytes(200)

{:ok, pid} = LRU.start_link(:my_cache, 1000)

LRU.put(:my_cache, :key1, binary)
LRU.put(:my_cache, :key2, binary)
LRU.put(:my_cache, :key3, binary)
LRU.put(:my_cache, :key4, binary)
LRU.put(:my_cache, :key5, binary)

:sys.get_state(pid)

LRU.put(:my_cache, :key6, :crypto.strong_rand_bytes(400))

:sys.get_state(pid)

assert nil == LRU.get(:my_cache, :key1)
assert nil == LRU.get(:my_cache, :key2)
end

test "should not cache a binary bigger than cache size" do
binary = get_a_binary_of_bytes(500)
binary = :crypto.strong_rand_bytes(500)

{:ok, pid} = LRU.start_link(:my_cache, 200)

Expand All @@ -89,7 +110,7 @@ defmodule ArchethicCache.LRUTest do
end

test "should remove all when purged" do
binary = get_a_binary_of_bytes(100)
binary = :crypto.strong_rand_bytes(100)

{:ok, _pid} = LRU.start_link(:my_cache, 500)

Expand Down Expand Up @@ -117,14 +138,4 @@ defmodule ArchethicCache.LRUTest do
assert "value1b" == LRU.get(:my_cache2, :key1)
end
end

defp get_a_binary_of_bytes(bytes) do
get_a_binary_of_bytes(bytes, <<>>)
end

defp get_a_binary_of_bytes(0, acc), do: acc

defp get_a_binary_of_bytes(bytes, acc) do
get_a_binary_of_bytes(bytes - 1, <<0::8, acc::binary>>)
end
end