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

[Dmitry Samersoff]

Coleen,

May be:

#if defined(__GNUC__) && (__GNUC__ >= 4)
   32   #define JNIEXPORT     __attribute__((visibility("default")))
   33   #define JNIIMPORT     __attribute__((visibility("default")))
#elif defined (__MSVC__)
   49   #define JNIEXPORT __declspec(dllexport)
   50   #define JNIIMPORT __declspec(dllimport)
   51   #define JNICALL __stdcall
#else
   35   #define JNIEXPORT
   36   #define JNIIMPORT
#endif


is better because these attributes is compiler but OS specific (thinking 
for example about BSD port).

-Dmitry


On 2011-01-24 23:00, Coleen Phillimore wrote:
> On 1/24/2011 2:58 PM, Dmitry Samersoff wrote:
>> Coleen,
>>
>> 1.
>> src/cpu/x86/vm/jni_x86.h
>>
>> JNIIMPORT,JNIEXPORT, JNICALL will not be defined under Win32 is it
>> intended?
> It is defined, just later on in the file. This code is under ifdef
> solaris && linux.
>>
>> 2. It may make sence to define JNIEXPORT_C extern "C" JNIEXPORT
> The callers usually declare with extern "C" if they want C linkage. I
> didn't want to change that.
>
> thanks,
> Coleen
>>
>> -Dmitry
>>
>>
>> On 2011-01-24 22:15, Coleen Phillimore wrote:
>>> Summary: Add option for gcc 4 and above, define JNIEXPORT and JNIIMPORT
>>>
>>> open webrev at http://cr.openjdk.java.net/~coleenp/6588413_2/
>>>
>>> This is an update to the code review. I found symbols that we should
>>> have exported. I also found exported symbols that were old and unused
>>> and removed them. Embedded question for Igor.
>>>
>>> Tested with some SA tests and others.
>>>
>>> Coleen
>>>
>>> On 1/21/2011 3:39 PM, Coleen Phillimore wrote:
>>>> On 1/21/2011 12:47 PM, Dmitry Samersoff wrote:
>>>>> Coleen,
>>>>>
>>>>> 1. Hotspot uses a mapfile to hide unused symbols -
>>>>> could we just change the mapfile?
>>>> Thanks Dmitry for this question because it prompted me to do more
>>>> searching and there were some definitions that should have JNIEXPORT
>>>> on them in the VM (AsyncGetCallTrace would have been broken!). The
>>>> option -fvisibility=hidden does more than hide the symbols as the
>>>> mapfile does, it also allows the compiler to optimize the address
>>>> calculations for internal functions it calls. I believe that is where
>>>> we get our performance improvements.
>>>>>
>>>>> 2. JNICALL is empty in both parts of #ifdef.
>>>>
>>>> Thanks, I fixed that.
>>>>
>>>> Coleen
>>>>>
>>>>> -Dmitry
>>>>>
>>>>>
>>>>> On 2011-01-21 19:57, 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
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>>
>


-- 
Dmitry Samersoff
Java Hotspot development team, SPB04
* There will come soft rains ...