RFR(S): 8241638: launcher time metrics alway report 1 on Linux when _JAVA_LAUNCHER_DEBUG set(Internet mail)

[David Holmes]

On 6/04/2020 11:50 am, David Holmes wrote:
> There is something not right here ...
> 
> On 4/04/2020 3:13 pm, Henry Jen wrote:
>> Internal test shows that inline implementation is not working for some 
>> Solaris artifacts, because the -DHAVE_GETHRTIME is not consistently 
>> defined, so it is actually broken. :)
>>
>>> [2020-04-03T15:59:26,981Z] Creating 
>>> support/test/hotspot/jtreg/native/bin/jvm-test-launcher from 1 file(s)
> 
> Are you sure that line actually pertains to any error? The test defines 
> a custom launcher which doesn't use libjli so should never be including 
> the header file we are discussing.
> 
>>> [2020-04-03T16:02:10,984Z]
>>> [2020-04-03T16:02:10,984Z] ERROR: Build failed for target 'default 
>>> (product-bundles test-bundles static-libs-bundles)' in configuration 
>>> 'solaris-sparcv9-open' (exit code 2)
>>> [2020-04-03T16:02:11,051Z]
>>> [2020-04-03T16:02:11,051Z] === Output from failing command(s) 
>>> repeated here ===
>>> [2020-04-03T16:02:11,055Z] * For target 
>>> support_native_java.base_libjli_BUILD_LIBJLI_link:
>>> [2020-04-03T16:02:11,061Z] Undefined            first referenced
>>> [2020-04-03T16:02:11,061Z]  symbol                  in file
>>> [2020-04-03T16:02:11,061Z] getTimeMicros                       
>>> /export/home/opt/mach5/mesos/work_dir/e101deec-9613-4d46-a64d-559e689496aa/workspace/build/solaris-sparcv9-open/support/native/java.base/libjli/java.o 
>>>
>>> [2020-04-03T16:02:11,061Z] ld: fatal: symbol referencing errors
> 
> This looks like a direct linkage error. AFAICS 
> ./launcher/LauncherCommon.gmk only defines -DHAVE_GETHRTIME for launcher 
> executables. But if we are building libjli it is not an executable. I'm 
> suspecting there is actually a long standing build bug here from when 
> libjli was introduced. Possibly only evident on an incremental build.

I can confirm that the flags set in LauncherCommon.gmk are not passed to 
the compilation of java_md_solinux.c (I added a custom -DDAVIDH to the 
linux flags and checked the build). So I have no idea how this has been 
working, if indeed it actually has.

David

> David
> -----
> 
>>> [2020-04-03T16:02:11,082Z]
>>> [2020-04-03T16:02:11,082Z] * All command lines available in 
>>> /export/home/opt/mach5/mesos/work_dir/e101deec-9613-4d46-a64d-559e689496aa/workspace/build/solaris-sparcv9-open/make-support/failure-logs. 
>>>
>>> [2020-04-03T16:02:11,086Z] === End of repeated output ===
>>> [2020-04-03T16:02:11,094Z]
>>> [2020-04-03T16:02:11,094Z] === Make failed targets repeated here ===
>>> [2020-04-03T16:02:11,736Z] CoreLibraries.gmk:206: recipe for target 
>>> '/export/home/opt/mach5/mesos/work_dir/e101deec-9613-4d46-a64d-559e689496aa/workspace/build/solaris-sparcv9-open/support/modules_libs/java.base/libjli.so' 
>>> failed
>>> [2020-04-03T16:02:11,737Z] make/Main.gmk:195: recipe for target 
>>> 'java.base-libs' failed
>>> [2020-04-03T16:02:11,739Z] === End of repeated output ===
>>> [2020-04-03T16:02:11,741Z]
>>
>>
>> I verified that either move implementation into .c as a function 
>> body[1] or change to #ifdef __solaris__[2] will fix that. So I think 
>> we will change to detect __solaris__ as webrev[2] rather than have an 
>> extra #define. If some other build want to have that, they can be 
>> modify that #ifdef easily.
>>
>> As I look into it, I found Mac have similar implementation with minor 
>> mistake, so I fixed that as well. Please review following based on 
>> zhanglin’s patch.
>>
>> I’ll push [2] once I got a +1.
>>
>> [1] http://cr.openjdk.java.net/~henryjen/jdk/8241638.0/webrev/
>> [2] http://cr.openjdk.java.net/~henryjen/jdk/8241638.1/webrev/
>>
>> Cheers,
>> Henry
>>
>>> On Apr 2, 2020, at 6:17 AM, Alan Bateman <Alan.Bateman at oracle.com> 
>>> wrote:
>>>
>>> On 02/04/2020 11:26, linzang(臧琳) wrote:
>>>> :
>>>>             Here is the updated webrev : 
>>>> http://cr.openjdk.java.net/~lzang/8241638/webrev04/
>>>>             Thanks for your help!
>>>>
>>> webrev04 looks good. My preference was to was to replace #ifdef 
>>> HAVE_GETHRTIME with #ifdef __solaris__ but there doesn't seem to be 
>>> appetite to do this now. I think Henry has offered to help sponsor.
>>>
>>> -Alan.
>>