On 08/02/2012 09:53 AM, levin li wrote:
> From: levin li <[email protected]>
> 
> 
> Signed-off-by: levin li <[email protected]>
> ---
>  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
-- 
sheepdog mailing list
[email protected]
http://lists.wpkg.org/mailman/listinfo/sheepdog

Reply via email to