inlining AllocateHeap()

Dmitry Samersoff dmitry.samersoff at oracle.com
Mon Mar 16 07:25:10 UTC 2015


David,

There are many cases when we might need compiler pragmas (e.g. to
suppress warnings)

So It might be a time to develop more generic solution rather than
muddle a code with pragma-related ifdefs.

*Can we create something like compiler_pragmas.hpp*

and put there

#define ALWAYS_INLINE()

#ifdef __GNUC__
#undef ALWAYS_INLINE
#define ALWAYS_INLINE()  __attribute__((always_inline))
#endif

etc.

than use ALWAYS_INLINE in the code when necessary.

-Dmitry
	



On 2015-03-16 03:44, David Holmes wrote:
> On 14/03/2015 9:29 AM, Coleen Phillimore wrote:
>>
>> 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.
> 
> Okay I'll follow Coleen's guidance on this. The original patch is fine.
> 
> Yasumasa you will need to file a CR and you will need a sponsor to push
> your changeset through JPRT once you have created it. I can do the
> latter, just email me the final changeset directly.
> 
> Thanks,
> David
> 
>> 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
>>>>>
>>>>>
>>


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.


More information about the hotspot-runtime-dev mailing list