[PATCH 2/3]fs/inode: iunique() Optimize Performance

[PATCH 2/3]fs/inode: iunique() Optimize Performance

[Liuweni]

---
Change log:
Change the hash operation from division to shift. It will cost less time.
Also, I change the divisor from L1_CACHE_BYTES to L1_CACHE_SHIFT.
In the cache.h, the most L1_CACHE_BYTES defined as "(1 << L1_CACHE_SHIFT)".
---
Signed-off-by: Liuwenyi<qingshenlwy@gmail.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Jan Kara <jack@suse.cz>
Cc: Nick Piggin <npiggin@suse.de>
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
diff --git a/fs/inode.c b/fs/inode.c
index 4d8e3be..397d65f 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -605,8 +605,8 @@ static unsigned long hash(struct super_block *sb, unsigned long hashval)
 {
 	unsigned long tmp;
 
-	tmp = (hashval * (unsigned long)sb) ^ (GOLDEN_RATIO_PRIME + hashval) /
-			L1_CACHE_BYTES;
+	tmp = (hashval * (unsigned long)sb) ^ (GOLDEN_RATIO_PRIME + hashval) >>
+			L1_CACHE_SHIFT;
 	tmp = tmp ^ ((tmp ^ GOLDEN_RATIO_PRIME) >> I_HASHBITS);
 	return tmp & I_HASHMASK;
 }

 				
--------------
Best Regards,
Liuweni
2009-11-25

Re: [PATCH 2/3]fs/inode: iunique() Optimize Performance

[Matthew Wilcox]

On Wed, Nov 25, 2009 at 10:12:19PM +0800, Liuweni wrote:
> @@ -605,8 +605,8 @@ static unsigned long hash(struct super_block *sb, unsigned long hashval)
>  {
>  	unsigned long tmp;
>  
> -	tmp = (hashval * (unsigned long)sb) ^ (GOLDEN_RATIO_PRIME + hashval) /
> -			L1_CACHE_BYTES;
> +	tmp = (hashval * (unsigned long)sb) ^ (GOLDEN_RATIO_PRIME + hashval) >>
> +			L1_CACHE_SHIFT;
>  	tmp = tmp ^ ((tmp ^ GOLDEN_RATIO_PRIME) >> I_HASHBITS);
>  	return tmp & I_HASHMASK;
>  }

Have you compared the compiler output before/after your change?  I'd be
amazed if GCC isn't able to optimise division-by-a-constant-power-of-two
into shift-by-constant.

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH 2/3]fs/inode: iunique() Optimize Performance

[Américo Wang]

On Wed, Nov 25, 2009 at 07:17:28AM -0700, Matthew Wilcox wrote:
>On Wed, Nov 25, 2009 at 10:12:19PM +0800, Liuweni wrote:
>> @@ -605,8 +605,8 @@ static unsigned long hash(struct super_block *sb, unsigned long hashval)
>>  {
>>  	unsigned long tmp;
>>  
>> -	tmp = (hashval * (unsigned long)sb) ^ (GOLDEN_RATIO_PRIME + hashval) /
>> -			L1_CACHE_BYTES;
>> +	tmp = (hashval * (unsigned long)sb) ^ (GOLDEN_RATIO_PRIME + hashval) >>
>> +			L1_CACHE_SHIFT;
>>  	tmp = tmp ^ ((tmp ^ GOLDEN_RATIO_PRIME) >> I_HASHBITS);
>>  	return tmp & I_HASHMASK;
>>  }
>
>Have you compared the compiler output before/after your change?  I'd be
>amazed if GCC isn't able to optimise division-by-a-constant-power-of-two
>into shift-by-constant.

If a compiler can't do this nowadays, I'd consider it's a bug.

-- 
Live like a child, think like the god.