Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@OliLay
OliLay committed Oct 26, 2023
1 parent 4905bc7 commit 14f5294
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 92 deletions.
15 changes: 7 additions & 8 deletions homcc/server/cache.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class Cache:
"""Path to the cache on the file system."""
max_size_bytes: int
"""Maximum size of the cache in bytes."""
current_size: int
current_size_bytes: int
"""Current size of the cache in bytes."""

def __init__(self, root_folder: Path, max_size_bytes: int):
Expand All @@ -33,7 +33,7 @@ def __init__(self, root_folder: Path, max_size_bytes: int):
self.cache: OrderedDict[str, str] = OrderedDict()
self.cache_mutex: Lock = Lock()
self.max_size_bytes = max_size_bytes
self.current_size = 0
self.current_size_bytes = 0

def _get_cache_file_path(self, hash_value: str) -> Path:
return self.cache_folder / hash_value
Expand All @@ -55,7 +55,7 @@ def _evict_oldest(self):
Evicts the oldest entry from the cache.
Note: The caller of this method has to ensure that the cache is locked.
"""
oldest_hash = next(iter(self.cache))
oldest_hash, _ = self.cache.popitem(last=False)
oldest_path = self._get_cache_file_path(oldest_hash)
oldest_size = oldest_path.stat().st_size

Expand All @@ -68,8 +68,7 @@ def _evict_oldest(self):
oldest_path,
)

self.current_size -= oldest_size
del self.cache[oldest_hash]
self.current_size_bytes -= oldest_size

@staticmethod
def _create_cache_folder(root_temp_folder: Path) -> Path:
Expand All @@ -92,17 +91,17 @@ def put(self, hash_value: str, content: bytearray):
logger.error(
"""File with hash '%s' can not be added to cache as it is larger than the maximum cache size.
(size in bytes: %i, max. cache size in bytes: %i)""",
hash,
hash_value,
len(content),
self.max_size_bytes,
)
raise RuntimeError("Cache size insufficient")

cached_file_path = self._get_cache_file_path(hash_value)
with self.cache_mutex:
while self.current_size + len(content) > self.max_size_bytes:
while self.current_size_bytes + len(content) > self.max_size_bytes:
self._evict_oldest()

Path.write_bytes(cached_file_path, content)
self.current_size += len(content)
self.current_size_bytes += len(content)
self.cache[hash_value] = str(cached_file_path)
163 changes: 79 additions & 84 deletions tests/server/cache_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,42 +5,39 @@
"""Test module for the server cache."""

from pathlib import Path
from tempfile import TemporaryDirectory

from homcc.server.cache import Cache


class TestCache:
"""Tests the server cache."""

def test_simple(self):
with TemporaryDirectory() as tmp_dir:
root_dir = Path(tmp_dir)
cache = Cache(root_dir, 1000)
cache_dir = root_dir / "cache"
def test_simple(self, tmp_path: Path):
cache = Cache(tmp_path, 1000)
cache_dir = tmp_path / "cache"

file1 = bytearray([0x1, 0x2, 0x3, 0x9])
cache.put("hash1", file1)
file1 = bytearray([0x1, 0x2, 0x3, 0x9])
cache.put("hash1", file1)

assert cache.get("hash1") == str(cache_dir / "hash1")
assert "hash1" in cache
assert Path.read_bytes(Path(cache.get("hash1"))) == file1
assert cache.get("hash1") == str(cache_dir / "hash1")
assert "hash1" in cache
assert Path.read_bytes(Path(cache.get("hash1"))) == file1

file2 = bytearray([0x3, 0x6, 0x3, 0x9])
cache.put("hash2", file2)
file2 = bytearray([0x3, 0x6, 0x3, 0x9])
cache.put("hash2", file2)

assert cache.get("hash2") == str(cache_dir / "hash2")
assert "hash2" in cache
assert Path.read_bytes(Path(cache.get("hash2"))) == file2
assert cache.get("hash2") == str(cache_dir / "hash2")
assert "hash2" in cache
assert Path.read_bytes(Path(cache.get("hash2"))) == file2

file3 = bytearray([0x4, 0x2])
cache.put("hash3", file3)
file3 = bytearray([0x4, 0x2])
cache.put("hash3", file3)

assert cache.get("hash3") == str(cache_dir / "hash3")
assert "hash3" in cache
assert Path.read_bytes(Path(cache.get("hash3"))) == file3
assert cache.get("hash3") == str(cache_dir / "hash3")
assert "hash3" in cache
assert Path.read_bytes(Path(cache.get("hash3"))) == file3

assert "other_hash" not in cache
assert "other_hash" not in cache

@staticmethod
def assert_hash_in_cache(cache: Cache, hash_value: str):
Expand All @@ -52,65 +49,63 @@ def assert_hash_not_in_cache(cache: Cache, hash_value: str):
assert hash_value not in cache
assert not (cache.cache_folder / hash_value).exists()

def test_eviction_size_limit(self):
with TemporaryDirectory() as tmp_dir:
cache = Cache(Path(tmp_dir), max_size_bytes=10)

cache.put("hash1", bytearray([0x1, 0x2, 0x3, 0x9]))
cache.put("hash2", bytearray([0x1, 0x2, 0x3, 0xA]))
cache.put("hash3", bytearray([0xFF, 0xFF]))
assert len(cache) == 3
self.assert_hash_in_cache(cache, "hash1")
self.assert_hash_in_cache(cache, "hash2")
self.assert_hash_in_cache(cache, "hash3")

cache.put("hash4", bytearray([0x1]))
assert len(cache) == 3
self.assert_hash_not_in_cache(cache, "hash1")
self.assert_hash_in_cache(cache, "hash2")
self.assert_hash_in_cache(cache, "hash3")
self.assert_hash_in_cache(cache, "hash4")

cache.put("hash5", bytearray([0x1]))
assert len(cache) == 4
self.assert_hash_in_cache(cache, "hash2")
self.assert_hash_in_cache(cache, "hash3")
self.assert_hash_in_cache(cache, "hash4")
self.assert_hash_in_cache(cache, "hash5")

cache.put("hash6", bytearray([0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9]))
assert len(cache) == 2
self.assert_hash_not_in_cache(cache, "hash2")
self.assert_hash_not_in_cache(cache, "hash3")
self.assert_hash_not_in_cache(cache, "hash4")
self.assert_hash_in_cache(cache, "hash5")
self.assert_hash_in_cache(cache, "hash6")

def test_eviction_order_lru(self):
with TemporaryDirectory() as tmp_dir:
cache = Cache(Path(tmp_dir), max_size_bytes=10)

cache.put("hash1", bytearray([0x1, 0x2, 0x3, 0x9]))
cache.put("hash2", bytearray([0x1, 0x2, 0x3, 0xA]))
cache.put("hash3", bytearray([0xFF, 0xFF]))
assert len(cache) == 3
self.assert_hash_in_cache(cache, "hash1")
self.assert_hash_in_cache(cache, "hash2")
self.assert_hash_in_cache(cache, "hash3")

cache.get("hash1") # make "hash1" the latest used element
cache.put("hash4", bytearray([0xFF, 0xFF, 0x0, 0x0]))
assert len(cache) == 3
self.assert_hash_not_in_cache(cache, "hash2")
self.assert_hash_in_cache(cache, "hash1")
self.assert_hash_in_cache(cache, "hash3")
self.assert_hash_in_cache(cache, "hash4")

assert "hash3" in cache # make "hash3" the latest used element
cache.put("hash5", bytearray([0xFF, 0xFF, 0x0, 0x0, 0xFF, 0xFF, 0x0, 0x0]))
assert len(cache) == 2
self.assert_hash_in_cache(cache, "hash3")
self.assert_hash_in_cache(cache, "hash5")
self.assert_hash_not_in_cache(cache, "hash1")
self.assert_hash_not_in_cache(cache, "hash2")
self.assert_hash_not_in_cache(cache, "hash4")
def test_eviction_size_limit(self, tmp_path: Path):
cache = Cache(tmp_path, max_size_bytes=10)

cache.put("hash1", bytearray([0x1, 0x2, 0x3, 0x9]))
cache.put("hash2", bytearray([0x1, 0x2, 0x3, 0xA]))
cache.put("hash3", bytearray([0xFF, 0xFF]))
assert len(cache) == 3
self.assert_hash_in_cache(cache, "hash1")
self.assert_hash_in_cache(cache, "hash2")
self.assert_hash_in_cache(cache, "hash3")

cache.put("hash4", bytearray([0x1]))
assert len(cache) == 3
self.assert_hash_not_in_cache(cache, "hash1")
self.assert_hash_in_cache(cache, "hash2")
self.assert_hash_in_cache(cache, "hash3")
self.assert_hash_in_cache(cache, "hash4")

cache.put("hash5", bytearray([0x1]))
assert len(cache) == 4
self.assert_hash_in_cache(cache, "hash2")
self.assert_hash_in_cache(cache, "hash3")
self.assert_hash_in_cache(cache, "hash4")
self.assert_hash_in_cache(cache, "hash5")

cache.put("hash6", bytearray([0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9]))
assert len(cache) == 2
self.assert_hash_not_in_cache(cache, "hash2")
self.assert_hash_not_in_cache(cache, "hash3")
self.assert_hash_not_in_cache(cache, "hash4")
self.assert_hash_in_cache(cache, "hash5")
self.assert_hash_in_cache(cache, "hash6")

def test_eviction_order_lru(self, tmp_path: Path):
cache = Cache(tmp_path, max_size_bytes=10)

cache.put("hash1", bytearray([0x1, 0x2, 0x3, 0x9]))
cache.put("hash2", bytearray([0x1, 0x2, 0x3, 0xA]))
cache.put("hash3", bytearray([0xFF, 0xFF]))
assert len(cache) == 3
self.assert_hash_in_cache(cache, "hash1")
self.assert_hash_in_cache(cache, "hash2")
self.assert_hash_in_cache(cache, "hash3")

cache.get("hash1") # make "hash1" the latest used element
cache.put("hash4", bytearray([0xFF, 0xFF, 0x0, 0x0]))
assert len(cache) == 3
self.assert_hash_not_in_cache(cache, "hash2")
self.assert_hash_in_cache(cache, "hash1")
self.assert_hash_in_cache(cache, "hash3")
self.assert_hash_in_cache(cache, "hash4")

assert "hash3" in cache # make "hash3" the latest used element
cache.put("hash5", bytearray([0xFF, 0xFF, 0x0, 0x0, 0xFF, 0xFF, 0x0, 0x0]))
assert len(cache) == 2
self.assert_hash_in_cache(cache, "hash3")
self.assert_hash_in_cache(cache, "hash5")
self.assert_hash_not_in_cache(cache, "hash1")
self.assert_hash_not_in_cache(cache, "hash2")
self.assert_hash_not_in_cache(cache, "hash4")

0 comments on commit 14f5294

Please sign in to comment.