[10] RFR(S) 8183262: noexecstack check in os::dll_load on Linux is too expensive

[Vladimir Kozlov]

Here is updated changes which converted ElfFile::specifies_noexecstack() 
to static method.

http://cr.openjdk.java.net/~kvn/8183262/webrev.01/

RBT testing passed.

AOT initialization time is still small - new code reads only first block 
from file.

Thanks,
Vladimir

On 6/30/17 3:52 PM, Vladimir Kozlov wrote:
> Thank you, Thomas
> 
> I thought about that but choose to do this since I will have to 
> duplicate checks done in constructor.
> 
> But starting looking on it I realized that my fix is incorrect because 
> load_tables() reads file header which is used by 
> specifies_noexecstack(). It is mess - os::dll_load() also reads header 
> later.
> 
> Anyway, to make my life easy I will duplicate header read code in 
> specifies_noexecstack().
> 
> I will send updated webrev after I test it.
> 
> Thanks,
> Vladimir
> 
> On 6/30/17 12:44 PM, Thomas Stüfe wrote:
>> Hi Vladimir,
>>
>> This seems fine, but I am a bit concerned that this misuses an
>> inconsistency in the API, namely the fact that specifies_noexecstack()
>> behaves differently than all other functions (re-read the file each 
>> time it
>> is called instead of using cached information). In order to understand 
>> the
>> "load_tables" argument in the constructor, one has to understand that odd
>> design, and know that load_tables=false means the only method which 
>> can be
>> used is specifies_noexecstack().
>>
>> As a proposal, could you instead make specifies_noexecstack() a static
>> function in ElfFile, such that:
>>
>> static bool ElfFile::specifies_noexecstack(const char* filename);
>>
>> Then it would be immediately clear that this function is to be used 
>> without
>> constructing the ElfFile object.
>>
>> Kind Regards, Thomas
>>
>> On Fri, Jun 30, 2017 at 7:07 PM, Vladimir Kozlov 
>> <vladimir.kozlov at oracle.com
>>> wrote:
>>
>>> webrev: http://cr.openjdk.java.net/~kvn/8183262/webrev/
>>> https://bugs.openjdk.java.net/browse/JDK-8183262
>>>
>>> To load AOT library we use os::dll_load() method provided on all 
>>> platforms.
>>>
>>> During investigation why AOT library load takes so long on Linux I found
>>> that it spend most of time in ElfFile::load_tables() method which 
>>> looks for
>>> string tables in library. It is called from ElfFile() constructor 
>>> used in
>>> os::dll_load() just to check if library is built with noexecstack:
>>>
>>>   if (os::uses_stack_guard_pages() && 
>>> !os::Linux::_stack_is_executable) {
>>>     ElfFile ef(filename);
>>>     if (!ef.specifies_noexecstack()) {
>>>
>>> There is no need for string tables load in such case. Avoiding loads 
>>> helps
>>> a lot:
>>>
>>> without fix:
>>>
>>> [0.047s][info][aot,startuptime] AOT initialization, 0.0424919 secs
>>>
>>> with fix:
>>>
>>> [0.002s][info][aot,startuptime] AOT initialization, 0.0002087 secs
>>>
>>>
>>> Note, JDK 9 is not affected because AOT uses system dlopen() since only
>>> Linux is supported.
>>>
>>> Thanks,
>>> Vladimir
>>>