[Sheepdog] [PATCH 1/2] add oid_to_vnodes() and obj_to_sheeps() to avoid too much vnodes traverse

[levin li]

On 05/15/2012 12:52 AM, Christoph Hellwig wrote:
> These patches should not just help with recovery performance but with
> anything looking up objects, so thanks a lot for doing this work.
>
> On Mon, May 14, 2012 at 12:12:35PM +0800, levin li wrote:
>> oid_to_vnode() and obj_to_sheep() traverse the vnode list to find the
>> target vnode, many times, sheep needs to call these two functions nr_copies
>> times, it means we also need to traverse the vnode list nr_copies times,
>> it's absolutely a waste, so I add these two functions to make it only
>> traverse one time in stead of calling obj_to_sheep()/oid_to_vnode() nr_copies
>> time.
> Any chance you could simply ckill of obj_to_sheep and oid_to_vnode?
>
> oid_to_vnode already has no caller left after your patch, and
> obj_to_sheep only one in collie and two in the recovery code.  Avoiding
> duplicate code paths helps maintainance over the long run.
>
I've noticed oid_to_vnode, I just keep it in case of future use,
as for collie, I think I just ignored it, I'd like to replace it with 
obj_to_sheeps,
but I don't think we should kill obj_to_sheep in recovery path, if we
use obj_to_sheeps there, it means we need more calling to get_nth_node,
I prefer to keep it.
> Also is there any benefit to keep hval_to_sheeps separate from
> obj_to_sheeps?  Especially if you move the fnv_64a_buf into the
> bsearch function it should be a lot cleaner than that.
>
> Either way these functions seem to large to be inlined to me, even
> before your changes.  Can we move them into out of lines functions
> to get better caching behaviour?  Especially with the many callers that
> might actually make a difference.
>
It's OK for me to make it cleaner and also make it not inlined,
I just kept it for respecting the old code, if no other comments,
I'd like to take your advise.

thanks,

levin