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

[Tom Rodriguez]

On Jan 21, 2011, at 12:35 PM, Coleen Phillimore wrote:

> 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.

The official versions of these files are in the JDK since they are Java interfaces, not hotspot interfaces.  The same updates we make in jni_md.h should be made in the official ones.  Also we should probably update the solaris definitions of JNIEXPORT as well since gcc can be used on Solaris too, even if we don't use it.

tom

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