inlining AllocateHeap()

[Coleen Phillimore]

There are other inline and noinline directives in allocation.hpp. We 
always assume that AllocateHeap and others are inlined.  NMT is touchy 
with respect to how it walks the stack and it took a bit of work and 
testing to get just the most useful frames saved.  I don't really want 
to risk this breaking!

I think the gcc directive is acceptable in this case.

Coleen


On 3/13/15, 9:16 AM, Yasumasa Suenaga wrote:
> 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
>>>
>>>