inlining AllocateHeap()

[Yasumasa Suenaga]

Hi,

> That would require more significant changes to NMT I think

I think two changes:

  1. Remove AllocateHeap(size_t, MEMFLAGS, AllocFailType) .
  2. Add "const NativeCallStack&" to argument of ReallocateHeap() .

I think that caller of AllocateHeap() and ReallocateHeap() should give PC to them.
However, it is significant changes.
Thus I proposed to add always_inline .


> I don't see how it will help if you have to know a-priori whether inlining has occurred or not. ??

I think we can use SA.
In case of Linux, sun.jvm.hotspot.debugger.linux.LinuxDebuggerLocal#lookup()
can lookup symbol from target process - we can check whether the function has been
inlined (cannot lookup) or not (can lookup).
So I think that we can write jtreg testcase.

BTW, should I file it to JBS?


Thanks,

Yasumasa


On 2015/03/13 17:35, David Holmes wrote:
> On 13/03/2015 6:13 PM, Thomas Stüfe wrote:
>> Hi Yasumasa, David,
>>
>> Maybe it would make sense to make the number-of-frames-to-skip-parameter
>> configurable?
>
> That would require more significant changes to NMT I think - plus I don't see how it will help if you have to know a-priori whether inlining has occurred or not. ??
>
> Thanks,
> David
>
>> Because the direct caller of AllocateHeap or os::malloc may also not be
>> interesting but still a generic wrapper. So, the user doing the
>> allocation trace could finetune this parameter to fit his needs.
>>
>> Thomas
>>
>>
>>
>> On Fri, Mar 13, 2015 at 6:40 AM, David Holmes <david.holmes at oracle.com
>> <mailto:david.holmes at oracle.com>> wrote:
>>
>>     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
>>     <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
>>
>>