Remove useless test and initialisation in name to hash computation
Hello,
I think there is a minor improvment that could be performed on name to hash computation in hash.c. Before first byte modulo 4 the computed value is reinitialised, but it is already correctly initialised before loop and after having processed last byte modulo 4. So the test and initialisation seems useless.
For the kernel, this lead to following change (sorry I do not have a git version of it, so it is a simple diff):
---------------------------------------------------------------
ext4/hash.c 2017-07-24 20:42:23.000000000 +0200 renamed from ext4/hash.c.orig 2017-07-24 20:41:53.000000000 +0200 +0 -4
@@ -79,8 +79,6 @@ static void str2hashbuf_signed(const cha if (len > num*4) len = num * 4; for (i = 0; i < len; i++) { - if ((i % 4) == 0) - val = pad; val = ((int) scp[i]) + (val << 8); if ((i % 4) == 3) { *buf++ = val;
@@ -107,8 +105,6 @@ static void str2hashbuf_unsigned(const c if (len > num*4) len = num * 4; for (i = 0; i < len; i++) { - if ((i % 4) == 0) - val = pad; val = ((int) ucp[i]) + (val << 8); if ((i % 4) == 3) { *buf++ = val;
---------------------------------------------------------------
For e2fsprogs, the 2 functions are combined in one, so there is only one change:
---------------------------------------------------------------
lib/ext2fs/dirhash.c +0 -2
@@ -154,8 +154,6 @@ static void str2hashbuf(const char *msg, int len, __u32 *buf, int num,if (len > num*4 ) len = num *4 ;for (i=0 ; i < len; i++) { -if ((i %4 ) ==0 ) - val = pad;if (unsigned_flag) c = (int ) ucp[i];else
---------------------------------------------------------------
Regards,
Damien
Quoted 48 lines · Damien Guibouret
Hello,
I think there is a minor improvment that could be performed on name to hash computation in
hash.c. Before first byte modulo 4 the computed value is reinitialised, but it is already correctly initialised before loop and after having processed last byte modulo 4. So the test and initialisation seems useless.For the kernel, this lead to following change (sorry I do not have a git version of it, so it is a simple diff):
---------------------------------------------------------------ext4/hash.c 2017-07-24 20:42:23.000000000 +0200 renamed from ext4/hash.c.orig 2017-07-24 20:41:53.000000000 +0200 +0
-5@@ -79,8 +79,6 @@ static void str2hashbuf_signed(const cha if (len > num*4) len = num * 4; for (i = 0; i < len; i++) { - if ((i % 4) == 0) - val = pad; val = ((int) scp[i]) + (val << 8); if ((i % 4) == 3) { *buf++ = val;@@ -107,8 +105,6 @@ static void str2hashbuf_unsigned(const c if (len > num*4) len = num * 4; for (i = 0; i < len; i++) { - if ((i % 4) == 0) - val = pad; val = ((int) ucp[i]) + (val << 8); if ((i % 4) == 3) { *buf++ = val; ---------------------------------------------------------------For e2fsprogs, the 2 functions are combined in one, so there is only one change:
---------------------------------------------------------------lib/ext2fs/dirhash.c +0
-2@@ -154,8 +154,6 @@ static void str2hashbuf(const char *msg, int len, __u32 *buf, int num, if (len > num*4) len = num * 4;for (i=0 ; i < len; i++) { -if ((i %4 ) ==0 ) - val = pad;if (unsigned_flag) c = (int ) ucp[i];
Quoted 2 lines
else
---------------------------------------------------------------
Hi Damien,
the change looks ok to me, have you done any testing to quantify the improvement, or validate the change ?
Quoted 1 line
From my limited testing the improvement seems to be around 11% for me
which would be nice to have. Also my test on 466k strings looks ok as well.
Are you willing to send out properly formated patch for kernel and e2fsprogs ?
Thanks!
-Lukas
Quoted 4 lines
Regards,
Damien
Quoted 60 lines · Lukas Czerner
Quoted 48 lines · Damien Guibouret
Hello,
I think there is a minor improvment that could be performed on name to hash computation in
hash.c. Before first byte modulo 4 the computed value is reinitialised, but it is already correctly initialised before loop and after having processed last byte modulo 4. So the test and initialisation seems useless.For the kernel, this lead to following change (sorry I do not have a git version of it, so it is a simple diff):
---------------------------------------------------------------ext4/hash.c 2017-07-24 20:42:23.000000000 +0200 renamed from ext4/hash.c.orig 2017-07-24 20:41:53.000000000 +0200 +0
-5@@ -79,8 +79,6 @@ static void str2hashbuf_signed(const cha if (len > num*4) len = num * 4; for (i = 0; i < len; i++) { - if ((i % 4) == 0) - val = pad; val = ((int) scp[i]) + (val << 8); if ((i % 4) == 3) { *buf++ = val;@@ -107,8 +105,6 @@ static void str2hashbuf_unsigned(const c if (len > num*4) len = num * 4; for (i = 0; i < len; i++) { - if ((i % 4) == 0) - val = pad; val = ((int) ucp[i]) + (val << 8); if ((i % 4) == 3) { *buf++ = val; ---------------------------------------------------------------For e2fsprogs, the 2 functions are combined in one, so there is only one change:
---------------------------------------------------------------lib/ext2fs/dirhash.c +0
-2@@ -154,8 +154,6 @@ static void str2hashbuf(const char *msg, int len, __u32 *buf, int num, if (len > num*4) len = num * 4;for (i=0 ; i < len; i++) { -if ((i %4 ) ==0 ) - val = pad;if (unsigned_flag) c = (int ) ucp[i];Quoted 2 lines
else
---------------------------------------------------------------Hi Damien,
the change looks ok to me, have you done any testing to quantify the improvement, or validate the change ?
Quoted 1 line
From my limited testing the improvement seems to be around 11% for me
Quoted 15 lines
which would be nice to have. Also my test on 466k strings looks ok as well.
Are you willing to send out properly formated patch for kernel and e2fsprogs ?
Thanks!
-LukasQuoted 3 lines
Regards,
Damien
Hello,
I just called old and new versions of functions with two different inputs (one multiple of 4 and one not) and check results were equals, nothing more. So not very formal, just a quick check but that allows covering all the code of these functions. For timing measurement I did nothing. I will see to prepare a more official patch this week-end.
Regards,
Damien