Request for review (S) 6588413: Use -fvisibility=hidden for gcc compiles

[Coleen Phillimore]

On 1/21/2011 3:30 PM, Tom Rodriguez wrote:
> On Jan 21, 2011, at 10:06 AM, Kelly O'Hair wrote:
>
>> Good stuff.
>>
>> Does this mean that GCC4 is now "required" to build hotspot? The compiler option change looks like
>> all versions of gcc will be getting it. I've been assuming that hotspot still needed to build with GCC3
>> for jdk6 and in general.
> It seems like this should be guarded in the makefile by CC_VER_MAJOR>= 4.

Yes, thanks, I missed this and made this change earlier today.
>> Also, There are copies of some of the vm files, like jni.h and jvm.h maybe? in the jdk repository,
>> and I think these are the ones that will show up in the jdk install image include directory.
>> Do they need to change too?
> I think they should.  There was a version of this bug filed against the jdk but it was duped into some bug about updating the required versions.  It might also be worth investigating building the libraries with -fvisibility=hidden too but I think the potential benefits there are more marginal, though the awt/2d kinds of libraries might win.
>
So I'm confused about this.   Are there copies of jni.hpp in the jdk 
repsitory.  All I see so far is #includes.  If the jdk files include the 
one in the vm, it will work fine with any visibility because JNIEXPORT 
is defined with visibility=default.  I did guard the definition with GCC 
version >= 4.  So I'm not sure what (Kelly) you are asking.
We could also compile the jdk with visibility=hidden and I could file a 
separate RFE for this.  In any case you need these definitions of 
JNIEXPORT and JNIIMPORT from the VM.  right?

>> Seems like this may have some implications to jni libraries, either binaries or in building them?
> The main point of the jni.h changes are to make sure that JNI writers can use -fvisibility=hidden without having to do extra work.  They probably are already but going forward it should be done automatically i think.  Other than that I don't think it affects jni libraries.

Yes, I think the definitions in jni.hpp is likely to be helpful (if this 
is the version that they magically get).

Thanks,
Coleen
> tom
>
>> -kto
>>
>> On Jan 21, 2011, at 8:57 AM, Coleen Phillimore wrote:
>>
>>> Summary: Add option for gcc 4 and above, define JNIEXPORT and JNIIMPORT to visibility=default, add for jio_snprintf and others since -fvisibility=hidden overrides --version-script definitions.
>>>
>>> Performance increases measured by Volker and Tom;
>>>
>>> 5% (JBB2005) and 2% (JVM98) on Linux/IA64 and 1,5% (JBB2005) and
>>> 0,5% (JVM98) on Linux/PPC64
>>>
>>>
>>> on intel hardware:
>>>
>>> specjvm98              15      314.61      0.00    0.33 0.006          Yes
>>>    javac                15      248.88      0.01    1.72 0.000          Yes
>>>    jack                 15      294.44      0.01    1.38 0.000          Yes
>>>
>>>
>>> open webrev at http://cr.openjdk.java.net/~coleenp/6588413/
>>> bug link at http://bugs.sun.com/view_bug.do?bug_id=6588413
>>>
>>> Tested with jmap for any serviceability impact, and tested hs_err_file decoding still works.
>>>
>>> Thanks,
>>> Coleen
>>>