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