2009-11-25 14:08:03

by liu weni

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

---
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<[email protected]>
Cc: Alexander Viro <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Jan Kara <[email protected]>
Cc: Nick Piggin <[email protected]>
Cc: [email protected]
Cc: [email protected]

---
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

2009-11-25 14:17:23

by Matthew Wilcox

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

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."

2009-11-25 14:44:58

by Cong Wang

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

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.