[sheepdog] [PATCH v1 3/3] lib: add cache version of writer and reader

[Liu Yuan]

On Mon, Feb 17, 2014 at 02:26:27PM +0800, Robin Dong wrote:
> From: Robin Dong <sanbai at taobao.com>
> 
> Add new functions to repleace default writer and reader so it can cache
> entire ext-node when set_vid_for_btree() need to access ext-node.
> 
> Signed-off-by: Robin Dong <sanbai at taobao.com>
> ---
>  lib/sd_inode.c      | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  sheep/http/oalloc.c |  4 +--
>  2 files changed, 98 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/sd_inode.c b/lib/sd_inode.c
> index 53035b0..809c3e5 100644
> --- a/lib/sd_inode.c
> +++ b/lib/sd_inode.c
> @@ -215,6 +215,92 @@ static void dump_btree(read_node_fn reader, struct sd_inode *inode)
>  #endif
>  }
>  
> +static write_node_fn raw_writer;
> +static read_node_fn raw_reader;
> +
> +#define NUMBER_OF_CACHE	4

Why 4? If no rantionale, then you should comment 'no rantionale'

>
> +struct data_object_cache {
> +	uint64_t oid;
> +	unsigned char mem[SD_INODE_DATA_INDEX_SIZE];
> +} cache_array[NUMBER_OF_CACHE];
> +static int cache_idx;
> +
> +static void cache_init(void)
> +{
> +	cache_idx = 0;
> +}
> +
> +static void cache_writeout(write_node_fn writer, int copies, int policy)
> +{

I think your cache is a transient cache that will be released after range set.
So better name it as cache_release(). And this is a inode cache, so would be
better use a prefix icache instead of a generic name 'cache'.

Does cache of the same inode support concurrent operations from different/same 
nodes? comment about it in the source.

> +	int i;
> +	for (i = 0; i < cache_idx; i++) {
> +		writer(cache_array[i].oid, cache_array[i].mem,
> +		       SD_INODE_DATA_INDEX_SIZE, 0, 0, copies, policy,
> +		       false, false);
> +	}
> +}
> +
> +static void cache_insert(write_node_fn writer, int copies, int policy,
> +			 uint64_t oid, void *mem)
> +{
> +	int i;
> +	for (i = 0; i < cache_idx; i++) {
> +		if (oid == cache_array[i].oid) {
> +			memcpy(cache_array[i].mem, mem,
> +			       SD_INODE_DATA_INDEX_SIZE);
> +			return;
> +		}
> +	}
> +
> +	if (cache_idx == (NUMBER_OF_CACHE - 1)) {
> +		sd_info("cache for B-tree is full, so write all out");
> +		cache_writeout(writer, copies, policy);
> +		cache_init();
> +	}
> +
> +	/* insert new cache */
> +	cache_array[cache_idx].oid = oid;
> +	memcpy(cache_array[cache_idx].mem, mem, SD_INODE_DATA_INDEX_SIZE);
> +	cache_idx++;
> +}
> +
> +static void *cache_find(uint64_t oid)
> +{
> +	int i;
> +	for (i = 0; i < cache_idx; i++) {
> +		if (cache_array[i].oid == oid)
> +			return cache_array[i].mem;
> +	}
> +	return NULL;
> +}
> +
> +static int writer_c(uint64_t id, void *mem, unsigned int len, uint64_t offset,
> +		    uint32_t flags, int copies, int copy_policy, bool create,
> +		    bool direct)

name it explicitly as 'cache_{reader,writer}'

Thanks
Yuan