[sheepdog] [PATCH] object cache: reuse the pulled buffer for read request

[Liu Yuan]

On 08/02/2012 09:53 AM, levin li wrote:
> From: levin li <xingke.lwp at taobao.com>
> 
> 
> Signed-off-by: levin li <xingke.lwp at taobao.com>
> ---
>  sheep/object_cache.c |   37 ++++++++++++++++++++++++++++++-------
>  1 files changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/sheep/object_cache.c b/sheep/object_cache.c
> index 3b6ecbe..ca95287 100644
> --- a/sheep/object_cache.c
> +++ b/sheep/object_cache.c
> @@ -740,7 +740,8 @@ out:
>  
>  
>  /* Fetch the object, cache it in success */
> -static int object_cache_pull(struct object_cache *oc, uint32_t idx)
> +static int object_cache_pull(struct object_cache *oc, uint32_t idx,
> +			     void **buffer)
>  {
>  	struct sd_req hdr;
>  	int ret = SD_RES_NO_MEM;
> @@ -776,7 +777,11 @@ static int object_cache_pull(struct object_cache *oc, uint32_t idx)
>  		else if (ret == SD_RES_OID_EXIST)
>  			ret = SD_RES_SUCCESS;
>  	}
> -	free(buf);
> +
> +	if (ret != SD_RES_SUCCESS)
> +		free(buf);
> +	else
> +		*buffer = buf;
>  out:
>  	return ret;
>  }
> @@ -952,6 +957,12 @@ int bypass_object_cache(struct request *req)
>  	return 0;
>  }
>  
> +static void read_cache_buffer(void *buffer, void *data, size_t count,
> +			      off_t offset)
> +{
> +	memcpy(data, (char *)buffer + offset, count);
> +}
> +
>  int object_cache_handle_request(struct request *req)
>  {
>  	struct sd_req *hdr = &req->rq;
> @@ -961,6 +972,7 @@ int object_cache_handle_request(struct request *req)
>  	struct object_cache *cache;
>  	struct object_cache_entry *entry;
>  	int ret, create = 0;
> +	void *data_buf = NULL;
>  
>  	dprintf("%08"PRIx32", len %"PRIu32", off %"PRIu64"\n", idx,
>  		hdr->data_length, hdr->obj.offset);
> @@ -973,7 +985,7 @@ int object_cache_handle_request(struct request *req)
>  retry:
>  	ret = object_cache_lookup(cache, idx, create);
>  	if (ret == SD_RES_NO_CACHE) {
> -		ret = object_cache_pull(cache, idx);
> +		ret = object_cache_pull(cache, idx, &data_buf);
>  		if (ret != SD_RES_SUCCESS)
>  			return ret;
>  	} else if (ret == SD_RES_EIO)
> @@ -983,6 +995,10 @@ retry:
>  	if (!entry) {
>  		dprintf("oid %"PRIx64" maybe reclaimed\n",
>  			idx_to_oid(vid, idx));
> +		if (data_buf) {
> +			free(data_buf);
> +			data_buf = NULL;
> +		}
>  		goto retry;
>  	}
>  
> @@ -994,16 +1010,23 @@ retry:
>  		update_cache_entry(cache, idx, hdr->data_length,
>  				   hdr->obj.offset);
>  	} else {
> -		ret = read_cache_object(cache->vid, idx, req->data,
> -					hdr->data_length, hdr->obj.offset);
> -		if (ret != SD_RES_SUCCESS)
> -			goto err;
> +		if (data_buf) {
> +			read_cache_buffer(data_buf, req->data, hdr->data_length,
> +					  hdr->obj.offset);
> +		} else {
> +			ret = read_cache_object(cache->vid, idx, req->data,
> +						hdr->data_length,
> +						hdr->obj.offset);

I think this trick doesn't improve the performance, because by default
we use buffered RW for object cache, which is mostly a memory operation.

Also this patch makes the code more complex. Consider we don't get much
benefit, I think we'd better drop this micro optimization.

Thanks,
Yuan