Last Comment Bug 415264 - Make Security use of new NSPR rotate macros
: Make Security use of new NSPR rotate macros
Status: RESOLVED FIXED
: perf
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: trunk
: x86 Windows 2000
: -- normal (vote)
: 3.12
Assigned To: Steve Snyder
:
Mentors:
Depends on: 331043
Blocks:
  Show dependency treegraph
 
Reported: 2008-02-01 10:34 PST by Steve Snyder
Modified: 2008-02-02 18:00 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Security use of NSPR rotate macros (2.02 KB, patch)
2008-02-01 10:34 PST, Steve Snyder
no flags Details | Diff | Splinter Review
Use the equivalent PR_ROTATE_LEFT32 calls (1.86 KB, patch)
2008-02-01 17:46 PST, Wan-Teh Chang
nelson: review+
Details | Diff | Splinter Review

Description Steve Snyder 2008-02-01 10:34:14 PST
Created attachment 300897 [details] [diff] [review]
Security use of NSPR rotate macros

Replace several uses of shift/xor, intended to rotate bits, with the use of the
new NSPR rotate macros.  The goal is to improve performance by replacing the
existing code with the hardware rotate instructions that the macros alias.
Comment 1 Wan-Teh Chang 2008-02-01 10:55:48 PST
Thanks for the patch.

Nelson already checked in the changes to mozilla/security/nss
(see bug 331043 comment 97).

MD4 is an old, broken hash algorithm.  It is not worth our time
to optimize our MD4 implementation.  So I'm not going to check
in the change to mozilla/security/manager/ssl/src/md4.c.
Comment 2 Wan-Teh Chang 2008-02-01 17:46:53 PST
Created attachment 300970 [details] [diff] [review]
Use the equivalent PR_ROTATE_LEFT32 calls

I found that all the code in Steve's patch is copies of this
in various forms:

    h = (h >> 28)) ^ (h << 4) ^ *s;

I believe that the original author of this code meant to say
"rotate h left by 4, and then XOR with the current character *s".
This is clearer in the copies of the code in JavaScript, where
it's written like this:

    h = (h >> (32 - 4)) ^ (h << 4) ^ *s;

So it is more natural to the equivalent PR_ROTATE_LEFT32 calls
here.  That is, I'd describe the rotation as "rotate left by
one hex digit" rather than "rotate right by seven hex digits."
Comment 3 Nelson Bolyard (seldom reads bugmail) 2008-02-01 18:05:16 PST
Comment on attachment 300970 [details] [diff] [review]
Use the equivalent PR_ROTATE_LEFT32 calls

r=nelson
Comment 4 Wan-Teh Chang 2008-02-02 18:00:48 PST
Comment on attachment 300970 [details] [diff] [review]
Use the equivalent PR_ROTATE_LEFT32 calls

I checked in this patch on the NSS trunk.

Checking in lib/base/hash.c;
/cvsroot/mozilla/security/nss/lib/base/hash.c,v  <--  hash.c
new revision: 1.11; previous revision: 1.10
done
Checking in lib/pki/pkistore.c;
/cvsroot/mozilla/security/nss/lib/pki/pkistore.c,v  <--  pkistore.c
new revision: 1.32; previous revision: 1.31
done

Note You need to log in before you can comment on or make changes to this bug.