You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
C++ performance improvements such as #885 and #934 need a reusable bounded cache. Without a production-ready cache abstraction, individual components may introduce unbounded per-object caches or duplicate cache implementations, making memory usage and ownership harder to control.
Current cache mechanisms
The repository currently contains several cache-related implementations:
cpp/src/common/cache/lru_cache.h provides a generic std::list + associative-map LRU skeleton with configurable maximum size and elasticity.
TsFileIOReader::device_node_cache_ is an active, reader-specific cache protected by a mutex. It is unbounded and backed by a shared PageArena, so individual eviction cannot reclaim the cached allocations.
The Java implementation has a working LRUCache based on LinkedHashMap, which can serve as a behavioral reference but cannot be reused directly by C++.
Problems with the existing C++ LRU
The generic C++ LRU is not currently used as a working production cache:
getCopy() and getRef() call an undefined get_nolock(), so these APIs fail when instantiated.
tryGetRef() copies the cached value into an output parameter despite its name. This is unsuitable for entries containing vectors or other expensive values.
The comments mention thread safety, but the implementation contains no synchronization.
MetadataQuerier passes std::mutex as the third template argument even though that argument is defined as the associative map type.
The cache operations in MetadataQuerier are commented out, and there are no focused C++ unit tests covering the cache behavior.
Proposed work
Define the cache contract clearly, including ownership, reference lifetime, capacity semantics, and whether synchronization is internal or the caller's responsibility.
Fix all public APIs and add a non-copying lookup mechanism, such as a pointer/reference accessor with a documented lifetime or a callback-based API.
Add unit tests covering insertion, lookup promotion, update, eviction, removal, clearing, bounded capacity, and disabled/unbounded configurations.
Correct or remove the inactive MetadataQuerier integration.
Motivation
C++ performance improvements such as #885 and #934 need a reusable bounded cache. Without a production-ready cache abstraction, individual components may introduce unbounded per-object caches or duplicate cache implementations, making memory usage and ownership harder to control.
Current cache mechanisms
The repository currently contains several cache-related implementations:
cpp/src/common/cache/lru_cache.hprovides a genericstd::list+ associative-map LRU skeleton with configurable maximum size and elasticity.TsFileIOReader::device_node_cache_is an active, reader-specific cache protected by a mutex. It is unbounded and backed by a sharedPageArena, so individual eviction cannot reclaim the cached allocations.LRUCachebased onLinkedHashMap, which can serve as a behavioral reference but cannot be reused directly by C++.Problems with the existing C++ LRU
The generic C++ LRU is not currently used as a working production cache:
getCopy()andgetRef()call an undefinedget_nolock(), so these APIs fail when instantiated.tryGetRef()copies the cached value into an output parameter despite its name. This is unsuitable for entries containing vectors or other expensive values.MetadataQuerierpassesstd::mutexas the third template argument even though that argument is defined as the associative map type.MetadataQuerierare commented out, and there are no focused C++ unit tests covering the cache behavior.Proposed work
MetadataQuerierintegration.Acceptance criteria
Related