Closed
Bug 424917
Opened 16 years ago
Closed 16 years ago
Performance regression with studio 12 compiler
Categories
(NSS :: Libraries, defect, P2)
Tracking
(Not tracked)
RESOLVED
FIXED
3.12
People
(Reporter: julien.pierre, Assigned: julien.pierre)
Details
Attachments
(1 file)
563 bytes,
patch
|
nelson
:
review+
|
Details | Diff | Splinter Review |
This problem occurs on x64. The compiler defaults changed and the RSA performance on AMD64 drops by 43% with the current compiler flags. We need to add -xprefetch=no to get back to the same level as we were getting with studio 11.
Assignee | ||
Comment 1•16 years ago
|
||
Only P2 because we aren't building officially with Studio 12 yet. But we may do so eventually.
Assignee: nobody → julien.pierre.boogz
Priority: -- → P2
Assignee | ||
Comment 2•16 years ago
|
||
Also see Sun CR 6637542 for what the compiler guys had to say.
Attachment #311508 -
Flags: review?(nelson)
Comment 3•16 years ago
|
||
Comment on attachment 311508 [details] [diff] [review] Add required option Questions and observations: > ASFLAGS += -xarch=generic64 -K PIC >+ CFLAGS += -xprefetch=no For consistency with the rest of this makefile, I'd say you want to use SOL_CFLAGS rather than CFLAGS. Do you only want to do this in this Makefile? Why not in coreconf? Do you want to do this for all Solaris sparc builds, including those with other versions of Studio? Do other versions of Studio ignore this option?
Assignee | ||
Comment 4•16 years ago
|
||
Nelson, In the absence of regular performance regression testing, I can't really say if we want to do this in all of coreconf or just in this Makefile. Basically, the compiler guys say this can help in some cases and hurt in others. And the compiler isn't smart enough to automatically figure out which ones :-( I do know we want do set this for freebl (actually, mpi) on AMD64 because I have personally tested this. I have not tested performance recently on Sparc. I'm OK with using SOL_CFLAGS instead of CFLAGS to set this.
Assignee | ||
Comment 5•16 years ago
|
||
Nelson, I also verified that -xprefetch=no was supported by studio 11 (and I believe it is the default there and has no effect).
Comment 6•16 years ago
|
||
How about studio 10? Do we claim to still support that compiler? What about earlier compilers? I'd really like it if this workaround could be applied only to the compiler version(s) that really need it. Basically, I don't want this workaround to have any detrimental effects on any other compilers.
Assignee | ||
Comment 7•16 years ago
|
||
We don't claim to support studio 10 anymore. We don't have different Makefile macros for the different versions of Studio compilers on Solaris. As I wrote the patch, even though it applies to both Studio 11 and 12, the switch has no effect on studio 11, and does what's needed for studio 12.
Comment 8•16 years ago
|
||
Comment on attachment 311508 [details] [diff] [review] Add required option OK, as long as support for Studio 10 is a non-issue, this patch is OK.
Attachment #311508 -
Flags: review?(nelson) → review+
Assignee | ||
Comment 9•16 years ago
|
||
Thanks for the review, Nelson. I made the change on the trunk with SOL_CFLAGS . Checking in Makefile; /cvsroot/mozilla/security/nss/lib/freebl/Makefile,v <-- Makefile new revision: 1.92; previous revision: 1.91 done
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 10•16 years ago
|
||
Christophe, To test this fix, use : rsaperf -n none -p 30 -t x (where x is the number of cores/CPUs) with and without the fix.
You need to log in
before you can comment on or make changes to this bug.
Description
•