[libcamera-devel] [PATCH 1/2] libcamera: V4L2BufferCache: Improve cache eviction strategy

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Sun, Feb 16, 2020 at 05:16:41PM +0100, Niklas Söderlund wrote:
> The strategy used to find a free cache entry in the first implementation
> was not the smartest, it picked the first free entry. This lead to
> unwanted performance issues as they cache was not used as good as it
> could for imported buffers.
> 
> Improve this by adding a last usage timestamp to the cache entries and
> change the eviction strategy to use the oldest free entry instead of the
> first one it finds.

Wouldn't it be simpler to as a LRU mechanism, to avoid searching in the
whole cache every time ?

> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/include/v4l2_videodevice.h | 1 +
>  src/libcamera/v4l2_videodevice.cpp       | 9 ++++++---
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h
> index fcf072641420dacf..f04feed87b24f28f 100644
> --- a/src/libcamera/include/v4l2_videodevice.h
> +++ b/src/libcamera/include/v4l2_videodevice.h
> @@ -125,6 +125,7 @@ private:
>  		bool operator==(const FrameBuffer &buffer);
>  
>  		bool free;
> +		utils::time_point lastUsed;
>  
>  	private:
>  		struct Plane {
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 99470ce11421c77c..9a4d2479b20f5e27 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -205,6 +205,7 @@ int V4L2BufferCache::get(const FrameBuffer &buffer)
>  {
>  	bool hit = false;
>  	int use = -1;
> +	utils::time_point oldest = utils::clock::now();
>  
>  	for (unsigned int index = 0; index < cache_.size(); index++) {
>  		const Entry &entry = cache_[index];
> @@ -212,8 +213,10 @@ int V4L2BufferCache::get(const FrameBuffer &buffer)
>  		if (!entry.free)
>  			continue;
>  
> -		if (use < 0)
> +		if (cache_[index].lastUsed < oldest) {
>  			use = index;
> +			oldest = cache_[index].lastUsed;
> +		}
>  
>  		/* Try to find a cache hit by comparing the planes. */
>  		if (cache_[index] == buffer) {
> @@ -245,12 +248,12 @@ void V4L2BufferCache::put(unsigned int index)
>  }
>  
>  V4L2BufferCache::Entry::Entry()
> -	: free(true)
> +	: free(true), lastUsed(utils::clock::now())
>  {
>  }
>  
>  V4L2BufferCache::Entry::Entry(bool free, const FrameBuffer &buffer)
> -	: free(free)
> +	: free(free), lastUsed(utils::clock::now())
>  {
>  	for (const FrameBuffer::Plane &plane : buffer.planes())
>  		planes_.emplace_back(plane);

-- 
Regards,

Laurent Pinchart