[Improvement][C++] Complete and adopt the existing LRU cache infrastructure
- Dominant language
- Java
- Stars
- 203
- Forks
- 104
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 33
Description
### 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.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
1. Define the cache contract clearly, including ownership, reference lifetime, capacity semantics, and whether synchronization is internal or the caller's responsibility.
2. 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.
3. Add unit tests covering insertion, lookup promotion, update, eviction, removal, clearing, bounded capacity, and disabled/unbounded configurations.
4. Correct or remove the inactive `MetadataQuerier` integration.
5. Adopt the completed cache in at least one real component, such as the bounded writer-level schema lookup cache discussed in #934.
6. Add benchmarks or memory measurements demonstrating that the cache improves the target workload while keeping additional memory bounded.
### Acceptance criteria
- All exposed cache APIs compile and have tested behavior.
- The eviction and capacity contract is deterministic and documented.
- Expensive cached values can be accessed without an unconditional copy.
- Thread-safety expectations and cached-value lifetime are explicit.
- At least one production C++ consumer uses the cache.
### Related
- #885
- #934
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by reading cpp/src/common/cache/lru_cache.h and the existing MetadataQuerier integration, then compare the Java LRUCache behavior described in the issue. Review TsFileIOReader::device_node_cache_ and the related cache usage before defining the contract and test scope. Done means the exposed APIs compile, focused C++ tests cover the listed behaviors, and a production consumer uses the bounded cache with documented lifetime and thread-safety expectations.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, java
- Domain
- backend, performance
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100