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

[David Holmes]

On 6/04/2020 10:53 am, linzang(臧琳) wrote:
>>      Certainly we can change to not inline by combining both fixes. I don’t think inline is necessary causing problem but that means header and the binary does need to match.
> 
> How about use macro instead of function?

No that's not appropriate use of a macro. This is a full function, if 
inlining may be problematic (and I agree the inlining may not be a 
problem afterall) then it should declared in the header file and defined 
in java_md_solinux.c

David

> +#define getTimeMicros() ({     \
> +    uint64_t result = 0; \
> +    struct timeval tv; \
> +    if (gettimeofday(&tv, NULL) != -1) { \
> +        result = 1000000LL * (uint64_t)tv.tv_sec; \
> +        result += (uint64_t)tv.tv_usec; \
> +    } \
> +    result; \
> +})
> 
> I have tested it works on linux, not sure whether it is ok for solaris.
>   
> BRs,
> Lin
> 
> On 2020/4/6, 3:37 AM, "Henry Jen" <henry.jen at oracle.com> wrote:
> 
>      
>      
>      > On Apr 5, 2020, at 6:52 AM, Alan Bateman <Alan.Bateman at oracle.com> wrote:
>      >
>      >
>      > On 05/04/2020 14:17, David Holmes wrote:
>      >> 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. :)
>      >>
>      >> The problem is in defining that function as inline rather than the -DHAVE_GETHRTIME.
>      >>
>      >>>> [2020-04-03T15:59:26,981Z] Creating support/test/hotspot/jtreg/native/bin/jvm-test-launcher from 1 file(s)
>      >>
>      >> The build rules for this special test launcher need to be examined as something seems wrong to me.
>      > I assume it's just that it's just compiled differently to the regular launchers and just not noticed that it was missing -DHAVE_GETHRTIME (unless for anyone to use it with _JAVA_LAUNCHER_DEBUG set).
>      
>      This is my understanding as well, we built something without define -DHAVA_GETHRTIME but linked with the library that was built with that defined and use inline function. We won’t notice this before because CounterGet is no-op before.
>      
>      > If we go with Henry's second webrev then I assume -DHAVE_GETHRTIME can be dropped from LauncherCommon.gmk to avoid confusing any further maintainers in this area.
>      
>      Right, I can drop that as well.
>      
>      > Also is there a strong need for the non-Solaris getTimeMicros to have be inline?
>      >
>      
>      Certainly we can change to not inline by combining both fixes. I don’t think inline is necessary causing problem but that means header and the binary does need to match.
>      
>      Cheers,
>      Henry
>      
>      
>      
>