[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: inlining AllocateHeap()
From: Yasumasa Suenaga <yasuenag () gmail ! com>
Date: 2015-03-28 4:43:23
Message-ID: 5516316B.6000100 () gmail ! com
[Download RAW message or body]
Sorry for the delay.
I filed it to JBS and uploaded webrev:
JDK-8076212: AllocateHeap() and ReallocateHeap() should be inlined.
http://cr.openjdk.java.net/~ysuenaga/JDK-8076212/webrev.00/
Could you review it?
> 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.
I'll send it to you after reviewing.
Yasumasa
On 2015年03月16日 09: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
>>>>>
>>>>>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic