Performance regression with studio 12 compiler



10 years ago
9 years ago


(Reporter: Julien Pierre, Assigned: Julien Pierre)


Firefox Tracking Flags

(Not tracked)



(1 attachment)

563 bytes, patch
Nelson Bolyard (seldom reads bugmail)
: review+
Details | Diff | Splinter Review


10 years ago
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.

Comment 1

10 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

Comment 2

10 years ago
Created attachment 311508 [details] [diff] [review]
Add required option

Also see Sun CR 6637542 for what the compiler guys had to say.
Attachment #311508 - Flags: review?(nelson)
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?

Comment 4

10 years ago

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.

Comment 5

10 years ago

I also verified that -xprefetch=no was supported by studio 11 (and I believe it is the default there and has no effect).
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.  

Comment 7

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

Comment 9

10 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
Last Resolved: 10 years ago
Resolution: --- → FIXED

Comment 10

9 years ago

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.