Request for Review: Backport of JDK-8206183: Possible construct EMPTY_STACK and allocation stack, etc. on first use.
Leslie Zhai
zhaixiang at loongson.cn
Mon Sep 17 14:30:57 UTC 2018
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: JDK-8206183-Backport-to-jdk8u-dev.patch
Type: text/x-patch
Size: 10821 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/jdk8u-dev/attachments/20180917/252904fb/JDK-8206183-Backport-to-jdk8u-dev-0001.patch>
More information about the jdk8u-dev
mailing list