inlining AllocateHeap()

[David Holmes]

Hi Yasumasa,

On 12/03/2015 9:58 PM, Yasumasa Suenaga wrote:
> Hi all,
>
> I tried to use NMT with details option on OpenJDK7 on RHEL6.6, but I got
> address at AllocateHeap() as malloc() caller.
>
> I checked symbol in libjvm.so <http://libjvm.so/> in OracleJDK8u40 Linux
> x64, it has AllocateHeap()
> symbol.
>
> AllocateHeap() is defined as inline function, and it gives CURRENT_PC to
> os::malloc(). I guess that implementation expects AllocateHeap() will be
> inlined.

It seems so.

> It may occur with GCC (g++) optimization only, however I want to fix it to
> analyze native memory with NMT on Linux.

According to the docs [1]:

"GCC does not inline any functions when not optimizing unless you 
specify the ‘always_inline’ attribute for the function"

> I applied patch as below. This patch makes AllocateHeap() as inline
> function.
> --------------
> diff -r af3b0db91659 src/share/vm/memory/allocation.inline.hpp
> --- a/src/share/vm/memory/allocation.inline.hpp Mon Mar 09 09:30:16 2015
> -0700
> +++ b/src/share/vm/memory/allocation.inline.hpp Thu Mar 12 20:45:57 2015
> +0900
> @@ -62,11 +62,18 @@
>     }
>     return p;
> }
> +
> +#ifdef __GNUC__
> +__attribute__((always_inline))
> +#endif

I dislike seeing the gcc specific directives in common code. I'm 
wondering whether we should perhaps only use CURRENT_PC in product (and 
optimized?) builds and use CALLER_PC otherwise. That would be imperfect 
of course It also makes me wonder whether the inlining is occurring as 
expected on other platforms.

I'd like to get other people's views on this.

Thanks,
David

[1] https://gcc.gnu.org/onlinedocs/gcc/Inline.html

> inline char* AllocateHeap(size_t size, MEMFLAGS flags,
>       AllocFailType alloc_failmode = AllocFailStrategy::EXIT_OOM) {
>     return AllocateHeap(size, flags, CURRENT_PC, alloc_failmode);
> }
>
> +#ifdef __GNUC__
> +__attribute__((always_inline))
> +#endif
> inline char* ReallocateHeap(char *old, size_t size, MEMFLAGS flag,
>       AllocFailType alloc_failmode = AllocFailStrategy::EXIT_OOM) {
>     char* p = (char*) os::realloc(old, size, flag, CURRENT_PC);
> --------------
>
> If this patch is accepted, I will file it to JBS and will upload webrev.
>
> Thanks,
>
> Yasumasa
>