diff --git a/include/aaruformat/consts.h b/include/aaruformat/consts.h index ade59ef..0433b7d 100644 --- a/include/aaruformat/consts.h +++ b/include/aaruformat/consts.h @@ -78,6 +78,10 @@ * still enabling efficient sequential and moderate random access patterns. */ #define MAX_CACHE_SIZE 536870912ULL +/** Maximum block header cache size (bytes). Block headers are a few dozen bytes each, so 8 MiB + * keeps headers cached for far more blocks than MAX_CACHE_SIZE can hold payloads for. */ +#define MAX_HEADER_CACHE_SIZE 8388608ULL + /** Size in bytes of the fixed LZMA properties header (lc/lp/pb + dictionary size). */ #define LZMA_PROPERTIES_LENGTH 5 diff --git a/include/aaruformat/lru.h b/include/aaruformat/lru.h index 24b7b68..5f6e8e4 100644 --- a/include/aaruformat/lru.h +++ b/include/aaruformat/lru.h @@ -5,6 +5,7 @@ #ifndef LIBAARUFORMAT_LRU_H #define LIBAARUFORMAT_LRU_H +#include #include #include @@ -24,29 +25,38 @@ struct CacheEntry { uint64_t key; ///< 64-bit integer key (unique within the cache). void *value; ///< Opaque value pointer associated with key. + size_t size; ///< Size in bytes of the memory pointed to by value. UT_hash_handle hh; ///< uthash handle (must remain per uthash docs). }; /** \struct CacheHeader * \brief Cache top-level descriptor encapsulating the hash table root and capacity limit. * - * The cache enforces an upper bound (max_items) on the number of tracked entries. On insert, - * the oldest entry is evicted when the limit is reached (LRU via uthash insertion order). + * The cache is bounded by the total number of *bytes* held by its values, not by the + * number of entries: the caches in this library store values of wildly different sizes + * (a block header is a few dozen bytes, a decompressed data block is megabytes), so an + * entry-count limit cannot express a memory budget. On insert, the least recently used + * entries are evicted until the cache is back under max_bytes. * * Fields: - * - max_items: Maximum number of entries allowed; 0 means unlimited. + * - max_bytes: Memory budget in bytes; 0 means unlimited. + * - cur_bytes: Bytes currently held by the cached values. * - cache: uthash root pointer; NULL when the cache is empty. * - free_func: Optional callback to free cached values on eviction/clear. + * + * The entry just inserted is never evicted, so a caller may keep using the pointer it + * handed over for the remainder of the call even if the value is larger than max_bytes. */ struct CacheHeader { - uint64_t max_items; ///< Hard limit for number of entries. + uint64_t max_bytes; ///< Memory budget in bytes for the cached values. 0 means unlimited. + uint64_t cur_bytes; ///< Bytes currently held by the cached values. struct CacheEntry *cache; ///< Hash root (uthash). NULL when empty. void (*free_func)(void *); ///< Optional callback to free cached values. NULL if not needed. }; void *find_in_cache_uint64(struct CacheHeader *cache, uint64_t key); -void add_to_cache_uint64(struct CacheHeader *cache, uint64_t key, void *value); +void add_to_cache_uint64(struct CacheHeader *cache, uint64_t key, void *value, size_t size); void free_cache(struct CacheHeader *cache); #endif // LIBAARUFORMAT_LRU_H diff --git a/src/create.c b/src/create.c index 9675187..a1fa0f3 100644 --- a/src/create.c +++ b/src/create.c @@ -402,10 +402,11 @@ AARU_EXPORT void *AARU_CALL aaruf_create(const char *filepath, const uint32_t me // Initialize caches TRACE("Initializing caches"); ctx->block_header_cache.cache = NULL; - const uint64_t cache_divisor = (uint64_t)ctx->image_info.SectorSize * (1ULL << ctx->shift); - ctx->block_header_cache.max_items = cache_divisor == 0 ? 0 : MAX_CACHE_SIZE / cache_divisor; + ctx->block_header_cache.cur_bytes = 0; + ctx->block_header_cache.max_bytes = MAX_HEADER_CACHE_SIZE; ctx->block_cache.cache = NULL; - ctx->block_cache.max_items = ctx->block_header_cache.max_items; + ctx->block_cache.cur_bytes = 0; + ctx->block_cache.max_bytes = MAX_CACHE_SIZE; // TODO: Cache tracks and sessions? diff --git a/src/erasure.c b/src/erasure.c index 67ede3f..27c04de 100644 --- a/src/erasure.c +++ b/src/erasure.c @@ -1507,14 +1507,14 @@ int32_t ec_recover_data_block(aaruformat_context *ctx, uint64_t block_offset, ui /* Cache the recovered block so subsequent sector reads from the same * block don't re-trigger recovery (this is the critical optimization). */ - add_to_cache_uint64(&ctx->block_cache, block_offset, block); + add_to_cache_uint64(&ctx->block_cache, block_offset, block, recovered_header.length); /* Also cache the recovered BlockHeader */ BlockHeader *cached_hdr = (BlockHeader *)malloc(sizeof(BlockHeader)); if(cached_hdr) { memcpy(cached_hdr, &recovered_header, sizeof(BlockHeader)); - add_to_cache_uint64(&ctx->block_header_cache, block_offset, cached_hdr); + add_to_cache_uint64(&ctx->block_header_cache, block_offset, cached_hdr, sizeof(BlockHeader)); } block = NULL; /* Ownership transferred to cache — don't free */ diff --git a/src/lru.c b/src/lru.c index bc5d99b..4ab638a 100644 --- a/src/lru.c +++ b/src/lru.c @@ -11,6 +11,23 @@ // eliminating all string conversion, malloc, and snprintf overhead // from the hot path. +/** + * @brief Frees a cache entry's value and unlinks it from the cache. + * + * @param cache Pointer to the cache header. + * @param entry Entry to drop. Must be linked in cache. + */ +static void drop_entry(struct CacheHeader *cache, struct CacheEntry *entry) +{ + HASH_DELETE(hh, cache->cache, entry); + + cache->cur_bytes -= entry->size; + + if(cache->free_func && entry->value) cache->free_func(entry->value); + + free(entry); +} + /** * @brief Finds a value in the cache by uint64_t key. * @@ -36,38 +53,39 @@ void *find_in_cache_uint64(struct CacheHeader *cache, const uint64_t key) } /** - * @brief Adds a value to the cache with a uint64_t key, evicting LRU if full. + * @brief Adds a value to the cache with a uint64_t key, evicting LRU entries if over budget. * - * Adds a new entry to the cache. If the cache exceeds its maximum size, - * evicts the least recently used (oldest insertion-order) entry. + * Adds a new entry to the cache. If the cache exceeds its memory budget, the least recently + * used entries are evicted until it fits again. The entry just inserted is never evicted, so the + * caller may keep using the pointer it handed over for the remainder of the call. * * @param cache Pointer to the cache header. * @param key 64-bit integer key to add. * @param value Pointer to the value to store. + * @param size Size in bytes of the memory pointed to by value. */ -void add_to_cache_uint64(struct CacheHeader *cache, const uint64_t key, void *value) +void add_to_cache_uint64(struct CacheHeader *cache, const uint64_t key, void *value, const size_t size) { struct CacheEntry *entry = malloc(sizeof(struct CacheEntry)); if(!entry) return; entry->key = key; entry->value = value; + entry->size = size; HASH_ADD(hh, cache->cache, key, sizeof(uint64_t), entry); + cache->cur_bytes += size; - // Evict oldest entry if cache exceeded capacity. - if(HASH_COUNT(cache->cache) > cache->max_items) + if(cache->max_bytes == 0) return; + + // Evict least recently used entries until back under budget. Never evict the entry just + // inserted: the caller still uses that pointer after this returns. + while(cache->cur_bytes > cache->max_bytes && HASH_COUNT(cache->cache) > 1) { - struct CacheEntry *tmp_entry; - HASH_ITER(hh, cache->cache, entry, tmp_entry) - { - HASH_DELETE(hh, cache->cache, entry); + struct CacheEntry *oldest = cache->cache; - if(cache->free_func && entry->value) - cache->free_func(entry->value); + if(oldest == NULL || oldest == entry) break; - free(entry); - break; - } + drop_entry(cache, oldest); } } @@ -85,15 +103,8 @@ void free_cache(struct CacheHeader *cache) if(!cache || !cache->cache) return; - HASH_ITER(hh, cache->cache, entry, tmp) - { - HASH_DELETE(hh, cache->cache, entry); + HASH_ITER(hh, cache->cache, entry, tmp) { drop_entry(cache, entry); } - if(cache->free_func && entry->value) - cache->free_func(entry->value); - - free(entry); - } - - cache->cache = NULL; + cache->cache = NULL; + cache->cur_bytes = 0; } diff --git a/src/open.c b/src/open.c index d2473d6..14cc212 100644 --- a/src/open.c +++ b/src/open.c @@ -718,17 +718,14 @@ AARU_EXPORT void *AARU_CALL aaruf_open(const char *filepath, const bool resume_m ctx->block_header_cache.free_func = free; ctx->block_cache.free_func = free; - const uint64_t cache_divisor = (uint64_t)ctx->image_info.SectorSize * (1ULL << ctx->shift); - if(cache_divisor == 0) - { - ctx->block_header_cache.max_items = 0; - ctx->block_cache.max_items = 0; - } - else - { - ctx->block_header_cache.max_items = MAX_CACHE_SIZE / cache_divisor; - ctx->block_cache.max_items = ctx->block_header_cache.max_items; - } + // Both caches are bounded by bytes held, so no block geometry is needed here. Deriving an + // entry count from a block size got this wrong: the divisor used ctx->shift, which is only + // ever set for DDT v1 images, so on every DDT v2 image the limit came out ~262144 entries of + // up to 8 MiB each and the cache grew until the whole decompressed image was resident. + ctx->block_header_cache.cur_bytes = 0; + ctx->block_cache.cur_bytes = 0; + ctx->block_header_cache.max_bytes = MAX_HEADER_CACHE_SIZE; + ctx->block_cache.max_bytes = MAX_CACHE_SIZE; // TODO: Cache tracks and sessions? diff --git a/src/read.c b/src/read.c index 0acdc47..ad10ff3 100644 --- a/src/read.c +++ b/src/read.c @@ -605,7 +605,7 @@ AARU_EXPORT int32_t AARU_CALL aaruf_read_sector(void *context, const uint64_t se } TRACE("Adding block header to cache"); - add_to_cache_uint64(&ctx->block_header_cache, block_offset, block_header); + add_to_cache_uint64(&ctx->block_header_cache, block_offset, block_header, sizeof(BlockHeader)); } else if(aaruf_fseek(ctx->imageStream, (aaru_off_t)(block_offset + sizeof(BlockHeader)), SEEK_SET) != 0) { @@ -1027,7 +1027,7 @@ AARU_EXPORT int32_t AARU_CALL aaruf_read_sector(void *context, const uint64_t se // Add block to cache TRACE("Adding block to cache"); - add_to_cache_uint64(&ctx->block_cache, block_offset, block); + add_to_cache_uint64(&ctx->block_cache, block_offset, block, block_header->length); memcpy(data, block + offset * block_header->sectorSize, block_header->sectorSize); *length = block_header->sectorSize;