RFR[8u] JDK-8206183: Possible construct EMPTY_STACK and allocation stack, etc. on first use.
Zhengyu Gu
zgu at redhat.com
Mon Sep 17 14:50:25 UTC 2018
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)
>>>>>
>>>
>
More information about the jdk8u-dev
mailing list