inlining AllocateHeap()

David Holmes david.holmes at oracle.com
Mon Mar 16 07:32:16 UTC 2015


Hi Dmitry,

On 16/03/2015 5:25 PM, Dmitry Samersoff wrote:
> 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.

Perhaps, but that seems a clear project in and of itself that would need 
to be resourced and prioritized (are you volunteering? :) )

It might be suitable for inclusion in the 
./share/vm/utilities/globalDefinitions_<compiler>.hpp headers.

David


> -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
>>>>>>
>>>>>>
>>>
>
>


More information about the hotspot-runtime-dev mailing list