RFR[8u] JDK-8206183: Possible construct EMPTY_STACK and allocation stack, etc. on first use.
Leslie Zhai
zhaixiang at loongson.cn
Mon Sep 17 15:03:17 UTC 2018
Hi Zhengyu,
Thanks for your review!
I will backport them individually tomorrow :)
在 2018年09月17日 23:00, Zhengyu Gu 写道:
> Hi Leslie,
>
> It looks like you combined JDK-8205965 and JDK-8206183 into one patch.
> However, this is not the backport should be done.
>
> Please backport them individually.
>
> Thanks,
>
> -Zhengyu
>
>
>
> On 09/17/2018 10:50 AM, Zhengyu Gu wrote:
>> Hi All,
>>
>> Please review Leslie's 8u backport:
>>
>> Webrev: http://cr.openjdk.java.net/~zgu/8206183_8u/webrev.00/
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8206183
>>
>> Thanks,
>>
>> -Zhengyu
>>
>>
>> On 09/17/2018 10:30 AM, Leslie Zhai wrote:
>>> Please generate webrev for me, thank you very much!
>>>
>>>
>>> 在 2018年09月17日 22:25, Zhengyu Gu 写道:
>>>> Would you be able to generate webrev? If not, could you attach the
>>>> patch file, I will generate it for you.
>>>>
>>>> -Zhengyu
>>>>
>>>> On 09/17/2018 10:16 AM, Leslie Zhai wrote:
>>>>> Hi Zhengyu,
>>>>>
>>>>> It is my pleasure!
>>>>>
>>>>> I am not a committer, please host it for me, thanks a lot!
>>>>>
>>>>> Today David also suggested to get someone to host the patch on
>>>>> cr.openjdk.java.net :)
>>>>>
>>>>> http://mail.openjdk.java.net/pipermail/jdk8u-dev/2018-September/007848.html
>>>>>
>>>>>
>>>>>
>>>>> 在 2018年09月17日 22:10, Zhengyu Gu 写道:
>>>>>> Hi Leslie,
>>>>>>
>>>>>> Thank you for doing this.
>>>>>>
>>>>>> Could you send me the formal webrev? I can host it for you and go
>>>>>> through formal review process.
>>>>>>
>>>>>> Are you a committer? If not, I can sponsor it once it is approved.
>>>>>>
>>>>>> -Zhengyu
>>>>>>
>>>>>>
>>>>>> On 09/17/2018 10:01 AM, Leslie Zhai wrote:
>>>>>>> Hi Zhengyu,
>>>>>>>
>>>>>>> I would like to backport the fix for:
>>>>>>>
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8206183
>>>>>>>
>>>>>>> To OpenJDK 8 updates dev:
>>>>>>>
>>>>>>> http://hg.openjdk.java.net/jdk8u/jdk8u-dev
>>>>>>>
>>>>>>> The fix is mostly the same as the version that was committed in 12.
>>>>>>>
>>>>>>> Please review the patch, thanks a lot!
>>>>>>>
>>>>>>> diff -r 16b9bbfaa450 src/share/vm/services/mallocSiteTable.cpp
>>>>>>> --- a/src/share/vm/services/mallocSiteTable.cpp Fri Sep 14
>>>>>>> 07:58:22 2018 -0700
>>>>>>> +++ b/src/share/vm/services/mallocSiteTable.cpp Mon Sep 17
>>>>>>> 21:44:58 2018 +0800
>>>>>>> @@ -28,34 +28,10 @@
>>>>>>> #include "runtime/atomic.hpp"
>>>>>>> #include "services/mallocSiteTable.hpp"
>>>>>>>
>>>>>>> -/*
>>>>>>> - * Early os::malloc() calls come from initializations of static
>>>>>>> variables, long before entering any
>>>>>>> - * VM code. Upon the arrival of the first os::malloc() call,
>>>>>>> malloc site hashtable has to be
>>>>>>> - * initialized, along with the allocation site for the
>>>>>>> hashtable entries.
>>>>>>> - * To ensure that malloc site hashtable can be initialized
>>>>>>> without triggering any additional os::malloc()
>>>>>>> - * call, the hashtable bucket array and hashtable entry
>>>>>>> allocation site have to be static.
>>>>>>> - * It is not a problem for hashtable bucket, since it is an
>>>>>>> array of pointer type, C runtime just
>>>>>>> - * allocates a block memory and zero the memory for it.
>>>>>>> - * But for hashtable entry allocation site object, things get
>>>>>>> tricky. C runtime not only allocates
>>>>>>> - * memory for it, but also calls its constructor at some later
>>>>>>> time. If we initialize the allocation site
>>>>>>> - * at the first os::malloc() call, the object will be
>>>>>>> reinitialized when its constructor is called
>>>>>>> - * by C runtime.
>>>>>>> - * To workaround above issue, we declare a static size_t array
>>>>>>> with the size of the CallsiteHashtableEntry,
>>>>>>> - * the memory is used to instantiate CallsiteHashtableEntry for
>>>>>>> the hashtable entry allocation site.
>>>>>>> - * Given it is a primitive type array, C runtime will do
>>>>>>> nothing other than assign the memory block for the variable,
>>>>>>> - * which is exactly what we want.
>>>>>>> - * The same trick is also applied to create NativeCallStack
>>>>>>> object for CallsiteHashtableEntry memory allocation.
>>>>>>> - *
>>>>>>> - * Note: C++ object usually aligns to particular alignment,
>>>>>>> depends on compiler implementation, we declare
>>>>>>> - * the memory as size_t arrays, to ensure the memory is aligned
>>>>>>> to native machine word alignment.
>>>>>>> - */
>>>>>>> -
>>>>>>> -// Reserve enough memory for NativeCallStack and
>>>>>>> MallocSiteHashtableEntry objects
>>>>>>> -size_t
>>>>>>> MallocSiteTable::_hash_entry_allocation_stack[CALC_OBJ_SIZE_IN_TYPE(NativeCallStack,
>>>>>>> size_t)];
>>>>>>> -size_t
>>>>>>> MallocSiteTable::_hash_entry_allocation_site[CALC_OBJ_SIZE_IN_TYPE(MallocSiteHashtableEntry,
>>>>>>> size_t)];
>>>>>>> -
>>>>>>> // Malloc site hashtable buckets
>>>>>>> MallocSiteHashtableEntry*
>>>>>>> MallocSiteTable::_table[MallocSiteTable::table_size];
>>>>>>> +const NativeCallStack*
>>>>>>> MallocSiteTable::_hash_entry_allocation_stack = NULL;
>>>>>>> +const MallocSiteHashtableEntry*
>>>>>>> MallocSiteTable::_hash_entry_allocation_site = NULL;
>>>>>>>
>>>>>>> // concurrent access counter
>>>>>>> volatile int MallocSiteTable::_access_count = 0;
>>>>>>> @@ -73,9 +49,6 @@
>>>>>>> * time, it is in single-threaded mode from JVM perspective.
>>>>>>> */
>>>>>>> bool MallocSiteTable::initialize() {
>>>>>>> - assert(sizeof(_hash_entry_allocation_stack) >=
>>>>>>> sizeof(NativeCallStack), "Sanity Check");
>>>>>>> - assert(sizeof(_hash_entry_allocation_site) >=
>>>>>>> sizeof(MallocSiteHashtableEntry),
>>>>>>> - "Sanity Check");
>>>>>>> assert((size_t)table_size <= MAX_MALLOCSITE_TABLE_SIZE,
>>>>>>> "Hashtable overflow");
>>>>>>>
>>>>>>> // Fake the call stack for hashtable entry allocation
>>>>>>> @@ -91,17 +64,19 @@
>>>>>>> }
>>>>>>> pc[0] = (address)MallocSiteTable::new_entry;
>>>>>>>
>>>>>>> - // Instantiate NativeCallStack object, have to use placement
>>>>>>> new operator. (see comments above)
>>>>>>> - NativeCallStack* stack = ::new
>>>>>>> ((void*)_hash_entry_allocation_stack)
>>>>>>> - NativeCallStack(pc, MIN2(((int)(sizeof(pc) /
>>>>>>> sizeof(address))), ((int)NMT_TrackingStackDepth)));
>>>>>>> + static const NativeCallStack stack(pc, MIN2(((int)(sizeof(pc)
>>>>>>> / sizeof(address))), ((int)NMT_TrackingStackDepth)));
>>>>>>> + static const MallocSiteHashtableEntry entry(stack, mtNMT);
>>>>>>>
>>>>>>> - // Instantiate hash entry for hashtable entry allocation
>>>>>>> callsite
>>>>>>> - MallocSiteHashtableEntry* entry = ::new
>>>>>>> ((void*)_hash_entry_allocation_site)
>>>>>>> - MallocSiteHashtableEntry(*stack, mtNMT);
>>>>>>> + assert(_hash_entry_allocation_stack == NULL &&
>>>>>>> + _hash_entry_allocation_site == NULL,
>>>>>>> + "Already initailized");
>>>>>>> +
>>>>>>> + _hash_entry_allocation_stack = &stack;
>>>>>>> + _hash_entry_allocation_site = &entry;
>>>>>>>
>>>>>>> // Add the allocation site to hashtable.
>>>>>>> - int index = hash_to_index(stack->hash());
>>>>>>> - _table[index] = entry;
>>>>>>> + int index = hash_to_index(stack.hash());
>>>>>>> + _table[index] = const_cast<MallocSiteHashtableEntry*>(&entry);
>>>>>>>
>>>>>>> return true;
>>>>>>> }
>>>>>>> @@ -204,6 +179,9 @@
>>>>>>> _table[index] = NULL;
>>>>>>> delete_linked_list(head);
>>>>>>> }
>>>>>>> +
>>>>>>> + _hash_entry_allocation_stack = NULL;
>>>>>>> + _hash_entry_allocation_site = NULL;
>>>>>>> }
>>>>>>>
>>>>>>> void
>>>>>>> MallocSiteTable::delete_linked_list(MallocSiteHashtableEntry*
>>>>>>> head) {
>>>>>>> @@ -211,7 +189,7 @@
>>>>>>> while (head != NULL) {
>>>>>>> p = head;
>>>>>>> head = (MallocSiteHashtableEntry*)head->next();
>>>>>>> - if (p !=
>>>>>>> (MallocSiteHashtableEntry*)_hash_entry_allocation_site) {
>>>>>>> + if (p != hash_entry_allocation_site()) {
>>>>>>> delete p;
>>>>>>> }
>>>>>>> }
>>>>>>> diff -r 16b9bbfaa450 src/share/vm/services/mallocSiteTable.hpp
>>>>>>> --- a/src/share/vm/services/mallocSiteTable.hpp Fri Sep 14
>>>>>>> 07:58:22 2018 -0700
>>>>>>> +++ b/src/share/vm/services/mallocSiteTable.hpp Mon Sep 17
>>>>>>> 21:44:58 2018 +0800
>>>>>>> @@ -42,7 +42,7 @@
>>>>>>>
>>>>>>> public:
>>>>>>> MallocSite() :
>>>>>>> - AllocationSite<MemoryCounter>(NativeCallStack::EMPTY_STACK),
>>>>>>> _flags(mtNone) {}
>>>>>>> + AllocationSite<MemoryCounter>(NativeCallStack::empty_stack()),
>>>>>>> _flags(mtNone) {}
>>>>>>>
>>>>>>> MallocSite(const NativeCallStack& stack, MEMFLAGS flags) :
>>>>>>> AllocationSite<MemoryCounter>(stack), _flags(flags) {}
>>>>>>> @@ -250,7 +250,13 @@
>>>>>>> }
>>>>>>>
>>>>>>> static inline const NativeCallStack*
>>>>>>> hash_entry_allocation_stack() {
>>>>>>> - return (NativeCallStack*)_hash_entry_allocation_stack;
>>>>>>> + assert(_hash_entry_allocation_stack != NULL, "Must be set");
>>>>>>> + return _hash_entry_allocation_stack;
>>>>>>> + }
>>>>>>> +
>>>>>>> + static inline const MallocSiteHashtableEntry*
>>>>>>> hash_entry_allocation_site() {
>>>>>>> + assert(_hash_entry_allocation_site != NULL, "Must be set");
>>>>>>> + return _hash_entry_allocation_site;
>>>>>>> }
>>>>>>>
>>>>>>> private:
>>>>>>> @@ -259,15 +265,11 @@
>>>>>>>
>>>>>>> // The callsite hashtable. It has to be a static table,
>>>>>>> // since malloc call can come from C runtime linker.
>>>>>>> - static MallocSiteHashtableEntry* _table[table_size];
>>>>>>> + static MallocSiteHashtableEntry* _table[table_size];
>>>>>>> + static const NativeCallStack* _hash_entry_allocation_stack;
>>>>>>> + static const MallocSiteHashtableEntry*
>>>>>>> _hash_entry_allocation_site;
>>>>>>>
>>>>>>>
>>>>>>> - // Reserve enough memory for placing the objects
>>>>>>> -
>>>>>>> - // The memory for hashtable entry allocation stack object
>>>>>>> - static size_t
>>>>>>> _hash_entry_allocation_stack[CALC_OBJ_SIZE_IN_TYPE(NativeCallStack,
>>>>>>> size_t)];
>>>>>>> - // The memory for hashtable entry allocation callsite object
>>>>>>> - static size_t
>>>>>>> _hash_entry_allocation_site[CALC_OBJ_SIZE_IN_TYPE(MallocSiteHashtableEntry,
>>>>>>> size_t)];
>>>>>>> NOT_PRODUCT(static int _peak_count;)
>>>>>>> };
>>>>>>>
>>>>>>> diff -r 16b9bbfaa450 src/share/vm/services/memTracker.cpp
>>>>>>> --- a/src/share/vm/services/memTracker.cpp Fri Sep 14
>>>>>>> 07:58:22 2018 -0700
>>>>>>> +++ b/src/share/vm/services/memTracker.cpp Mon Sep 17
>>>>>>> 21:44:58 2018 +0800
>>>>>>> @@ -67,10 +67,6 @@
>>>>>>> os::unsetenv(buf);
>>>>>>> }
>>>>>>>
>>>>>>> - // Construct NativeCallStack::EMPTY_STACK. It may get
>>>>>>> constructed twice,
>>>>>>> - // but it is benign, the results are the same.
>>>>>>> - ::new ((void*)&NativeCallStack::EMPTY_STACK)
>>>>>>> NativeCallStack(0, false);
>>>>>>> -
>>>>>>> if (!MallocTracker::initialize(level) ||
>>>>>>> !VirtualMemoryTracker::initialize(level)) {
>>>>>>> level = NMT_off;
>>>>>>> diff -r 16b9bbfaa450 src/share/vm/services/memTracker.hpp
>>>>>>> --- a/src/share/vm/services/memTracker.hpp Fri Sep 14
>>>>>>> 07:58:22 2018 -0700
>>>>>>> +++ b/src/share/vm/services/memTracker.hpp Mon Sep 17
>>>>>>> 21:44:58 2018 +0800
>>>>>>> @@ -31,8 +31,8 @@
>>>>>>>
>>>>>>> #if !INCLUDE_NMT
>>>>>>>
>>>>>>> -#define CURRENT_PC NativeCallStack::EMPTY_STACK
>>>>>>> -#define CALLER_PC NativeCallStack::EMPTY_STACK
>>>>>>> +#define CURRENT_PC NativeCallStack::empty_stack()
>>>>>>> +#define CALLER_PC NativeCallStack::empty_stack()
>>>>>>>
>>>>>>> class Tracker : public StackObj {
>>>>>>> public:
>>>>>>> @@ -83,9 +83,9 @@
>>>>>>> extern volatile bool NMT_stack_walkable;
>>>>>>>
>>>>>>> #define CURRENT_PC ((MemTracker::tracking_level() ==
>>>>>>> NMT_detail && NMT_stack_walkable) ? \
>>>>>>> - NativeCallStack(0, true) :
>>>>>>> NativeCallStack::EMPTY_STACK)
>>>>>>> + NativeCallStack(0, true) :
>>>>>>> NativeCallStack::empty_stack())
>>>>>>> #define CALLER_PC ((MemTracker::tracking_level() ==
>>>>>>> NMT_detail && NMT_stack_walkable) ? \
>>>>>>> - NativeCallStack(1, true) :
>>>>>>> NativeCallStack::EMPTY_STACK)
>>>>>>> + NativeCallStack(1, true) :
>>>>>>> NativeCallStack::empty_stack())
>>>>>>>
>>>>>>> class MemBaseline;
>>>>>>> class Mutex;
>>>>>>> diff -r 16b9bbfaa450 src/share/vm/services/virtualMemoryTracker.cpp
>>>>>>> --- a/src/share/vm/services/virtualMemoryTracker.cpp Fri Sep 14
>>>>>>> 07:58:22 2018 -0700
>>>>>>> +++ b/src/share/vm/services/virtualMemoryTracker.cpp Mon Sep 17
>>>>>>> 21:44:58 2018 +0800
>>>>>>> @@ -167,7 +167,7 @@
>>>>>>> // higher part
>>>>>>> address high_base = addr + sz;
>>>>>>> size_t high_size = top - high_base;
>>>>>>> - CommittedMemoryRegion high_rgn(high_base, high_size,
>>>>>>> NativeCallStack::EMPTY_STACK);
>>>>>>> + CommittedMemoryRegion high_rgn(high_base, high_size,
>>>>>>> NativeCallStack::empty_stack());
>>>>>>> return add_committed_region(high_rgn);
>>>>>>> } else {
>>>>>>> return false;
>>>>>>> diff -r 16b9bbfaa450 src/share/vm/services/virtualMemoryTracker.hpp
>>>>>>> --- a/src/share/vm/services/virtualMemoryTracker.hpp Fri Sep 14
>>>>>>> 07:58:22 2018 -0700
>>>>>>> +++ b/src/share/vm/services/virtualMemoryTracker.hpp Mon Sep 17
>>>>>>> 21:44:58 2018 +0800
>>>>>>> @@ -305,7 +305,7 @@
>>>>>>>
>>>>>>>
>>>>>>> ReservedMemoryRegion(address base, size_t size) :
>>>>>>> - VirtualMemoryRegion(base, size),
>>>>>>> _stack(NativeCallStack::EMPTY_STACK), _flag(mtNone),
>>>>>>> + VirtualMemoryRegion(base, size),
>>>>>>> _stack(NativeCallStack::empty_stack()), _flag(mtNone),
>>>>>>> _all_committed(false) { }
>>>>>>>
>>>>>>> // Copy constructor
>>>>>>> diff -r 16b9bbfaa450 src/share/vm/utilities/nativeCallStack.cpp
>>>>>>> --- a/src/share/vm/utilities/nativeCallStack.cpp Fri Sep 14
>>>>>>> 07:58:22 2018 -0700
>>>>>>> +++ b/src/share/vm/utilities/nativeCallStack.cpp Mon Sep 17
>>>>>>> 21:44:58 2018 +0800
>>>>>>> @@ -27,8 +27,6 @@
>>>>>>> #include "utilities/globalDefinitions.hpp"
>>>>>>> #include "utilities/nativeCallStack.hpp"
>>>>>>>
>>>>>>> -const NativeCallStack NativeCallStack::EMPTY_STACK(0, false);
>>>>>>> -
>>>>>>> NativeCallStack::NativeCallStack(int toSkip, bool fillStack) :
>>>>>>> _hash_value(0) {
>>>>>>>
>>>>>>> diff -r 16b9bbfaa450 src/share/vm/utilities/nativeCallStack.hpp
>>>>>>> --- a/src/share/vm/utilities/nativeCallStack.hpp Fri Sep 14
>>>>>>> 07:58:22 2018 -0700
>>>>>>> +++ b/src/share/vm/utilities/nativeCallStack.hpp Mon Sep 17
>>>>>>> 21:44:58 2018 +0800
>>>>>>> @@ -52,9 +52,6 @@
>>>>>>> * from it.
>>>>>>> */
>>>>>>> class NativeCallStack : public StackObj {
>>>>>>> - public:
>>>>>>> - static const NativeCallStack EMPTY_STACK;
>>>>>>> -
>>>>>>> private:
>>>>>>> address _stack[NMT_TrackingStackDepth];
>>>>>>> unsigned int _hash_value;
>>>>>>> @@ -63,6 +60,10 @@
>>>>>>> NativeCallStack(int toSkip = 0, bool fillStack = false);
>>>>>>> NativeCallStack(address* pc, int frameCount);
>>>>>>>
>>>>>>> + static inline const NativeCallStack& empty_stack() {
>>>>>>> + static const NativeCallStack EMPTY_STACK(0, false);
>>>>>>> + return EMPTY_STACK;
>>>>>>> + }
>>>>>>>
>>>>>>> // if it is an empty stack
>>>>>>> inline bool is_empty() const {
>>>>>>>
>>>>>>> ----- 8< -------- 8< -------- 8< -------- 8< -------- 8<
>>>>>>> -------- 8< ---
>>>>>>>
>>>>>>> Applied the patch, then built with clang-8:
>>>>>>>
>>>>>>> $ strings
>>>>>>> build/linux-x86_64-normal-server-fastdebug/images/j2sdk-image/bin/java
>>>>>>> | grep clang
>>>>>>> LLVM China clang version 8.0.0
>>>>>>> (git at github.com:llvm-mirror/clang.git
>>>>>>> 7f223b8fbf26fa0e4d8f98847a53c4ba457720f0)
>>>>>>> (git at github.com:llvm-mirror/llvm.git
>>>>>>> 841e300fb15be4f9931d18d2f24f48cb59ef24a8) (based on LLVM 8.0.0svn)
>>>>>>>
>>>>>>> $
>>>>>>> ./build/linux-x86_64-normal-server-fastdebug/images/j2sdk-image/bin/java
>>>>>>> -version
>>>>>>>
>>>>>>> # To suppress the following error report, specify this argument
>>>>>>> # after -XX: or in .hotspotrc:
>>>>>>> SuppressErrorAt=/os_linux_x86.cpp:882
>>>>>>> #
>>>>>>> # A fatal error has been detected by the Java Runtime Environment:
>>>>>>> #
>>>>>>> # Internal Error
>>>>>>> (/home/xiangzhai/project/jdk8u-llvm-dev/hotspot/src/os_cpu/linux_x86/vm/os_linux_x86.cpp:882),
>>>>>>> pid=16503, tid=0x00007f532ceaf700
>>>>>>> # assert(((intptr_t)os::current_stack_pointer() &
>>>>>>> (StackAlignmentInBytes-1)) == 0) failed: incorrect stack alignment
>>>>>>>
>>>>>>> Just workaround for JDK-8186780:
>>>>>>>
>>>>>>> diff -r 16b9bbfaa450 src/os_cpu/linux_x86/vm/os_linux_x86.cpp
>>>>>>> --- a/src/os_cpu/linux_x86/vm/os_linux_x86.cpp Fri Sep 14
>>>>>>> 07:58:22 2018 -0700
>>>>>>> +++ b/src/os_cpu/linux_x86/vm/os_linux_x86.cpp Mon Sep 17
>>>>>>> 21:54:24 2018 +0800
>>>>>>> @@ -878,7 +878,7 @@
>>>>>>>
>>>>>>> #ifndef PRODUCT
>>>>>>> void os::verify_stack_alignment() {
>>>>>>> -#ifdef AMD64
>>>>>>> +#if defined(AMD64) && !defined(__clang__)
>>>>>>> assert(((intptr_t)os::current_stack_pointer() &
>>>>>>> (StackAlignmentInBytes-1)) == 0, "incorrect stack alignment");
>>>>>>> #endif
>>>>>>> }
>>>>>>>
>>>>>>> ----- 8< -------- 8< -------- 8< -------- 8< -------- 8<
>>>>>>> -------- 8< ---
>>>>>>>
>>>>>>> $
>>>>>>> ./build/linux-x86_64-normal-server-fastdebug/images/j2sdk-image/bin/java
>>>>>>> -version
>>>>>>>
>>>>>>> openjdk version "1.8.0-internal-fastdebug"
>>>>>>> OpenJDK Runtime Environment (build
>>>>>>> 1.8.0-internal-fastdebug-xiangzhai_2018_09_15_19_30-b00)
>>>>>>> OpenJDK 64-Bit Server VM (build 25.71-b00-fastdebug, mixed mode)
>>>>>>>
>>>>>
>>>
--
Regards,
Leslie Zhai
More information about the jdk8u-dev
mailing list