Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8251158: Implementation of JEP 387: Elastic Metaspace #336

Closed
wants to merge 29 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
a7dc085
Review version 2 (2020-09-04)
tstuefe Sep 19, 2020
731f795
JEP387 review feedback round 2
tstuefe Sep 19, 2020
b5eaf32
Remove trailing whitespaces
tstuefe Sep 24, 2020
9f68bab
Remove empty lines from include sections
tstuefe Sep 25, 2020
267ce0a
Style fixes
tstuefe Sep 29, 2020
0e41494
Make MetaspaceGuardAllocations a diagnostic flag
tstuefe Sep 29, 2020
e64d8f0
Make MetaspaceGuardAllocations a diagnostic flag (2)
tstuefe Sep 29, 2020
20048f9
cds MaxMetaspaceSize test does not need MaxMetaspaceSize increase
tstuefe Sep 30, 2020
60b494d
Review feedback Ioi
tstuefe Sep 30, 2020
1dbb500
Create submit.yml
tstuefe Sep 30, 2020
ec9f7d3
Merge branch 'jep387' of github.com:tstuefe/jdk into jep387
tstuefe Sep 30, 2020
d1e413b
Remove accidentally added file
tstuefe Sep 30, 2020
f5cf615
Merge branch 'master' into jep387
tstuefe Sep 30, 2020
f1cebb0
Move ClassLoaderMetaspace to own include and correct includes
tstuefe Oct 2, 2020
ece884f
Misc. Review feedback
tstuefe Oct 2, 2020
4cc94c5
Remove some empty lines
tstuefe Oct 4, 2020
128c494
Review Feedback Richard 1
tstuefe Oct 5, 2020
b90a100
Fix 32bit build
tstuefe Oct 6, 2020
eacafe4
Review Feedback Richard 2
tstuefe Oct 6, 2020
7658f97
Assert that in reclaim=none mode new chunks are fully committed
tstuefe Oct 9, 2020
bd0e7f4
Loosen unnecessarily strict restriction of allowed chunk states on Ch…
tstuefe Oct 9, 2020
2c97008
Fix retrieval from FreeChunkList; add test; update comments
tstuefe Oct 11, 2020
b6e8b84
Add ASCII art to better describe Metachunk relationship to their payload
tstuefe Oct 11, 2020
b9df951
Richard feedback 3
tstuefe Oct 12, 2020
e895bfa
Merge
tstuefe Oct 12, 2020
df2f81d
Merge
tstuefe Oct 15, 2020
c5dbfbc
Merge
tstuefe Oct 17, 2020
973e7aa
Small include file correction
tstuefe Oct 17, 2020
1b52124
Fix Linux 32bit link issue
tstuefe Oct 19, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -58,7 +58,9 @@
#include "logging/log.hpp"
#include "logging/logStream.hpp"
#include "memory/allocation.inline.hpp"
#include "memory/classLoaderMetaspace.hpp"
#include "memory/metadataFactory.hpp"
#include "memory/metaspace.hpp"
#include "memory/resourceArea.hpp"
#include "memory/universe.hpp"
#include "oops/access.inline.hpp"
@@ -953,9 +955,11 @@ void ClassLoaderData::verify() {
guarantee(cl != NULL || this == ClassLoaderData::the_null_class_loader_data() || has_class_mirror_holder(), "must be");

// Verify the integrity of the allocated space.
#ifdef ASSERT
if (metaspace_or_null() != NULL) {
metaspace_or_null()->verify();
}
#endif

for (Klass* k = _klasses; k != NULL; k = k->next_link()) {
guarantee(k->class_loader_data() == this, "Must be the same");
@@ -62,6 +62,7 @@ class ModuleEntryTable;
class PackageEntryTable;
class DictionaryEntry;
class Dictionary;
class ClassLoaderMetaspace;

// ClassLoaderData class

@@ -661,24 +661,6 @@ Klass* ClassLoaderDataGraphKlassIteratorAtomic::next_klass() {
return NULL;
}

ClassLoaderDataGraphMetaspaceIterator::ClassLoaderDataGraphMetaspaceIterator() {
assert(SafepointSynchronize::is_at_safepoint(), "must be at safepoint!");
_data = ClassLoaderDataGraph::_head;
}

ClassLoaderDataGraphMetaspaceIterator::~ClassLoaderDataGraphMetaspaceIterator() {}

ClassLoaderMetaspace* ClassLoaderDataGraphMetaspaceIterator::get_next() {
assert(_data != NULL, "Should not be NULL in call to the iterator");
ClassLoaderMetaspace* result = _data->metaspace_or_null();
_data = _data->next();
// This result might be NULL for class loaders without metaspace
// yet. It would be nice to return only non-null results but
// there is no guarantee that there will be a non-null result
// down the list so the caller is going to have to check.
return result;
}

void ClassLoaderDataGraph::verify() {
ClassLoaderDataGraphIterator iter;
while (ClassLoaderData* cld = iter.get_next()) {
@@ -162,12 +162,4 @@ class ClassLoaderDataGraphKlassIteratorAtomic : public StackObj {
static Klass* next_klass_in_cldg(Klass* klass);
};

class ClassLoaderDataGraphMetaspaceIterator : public StackObj {
ClassLoaderData* _data;
public:
ClassLoaderDataGraphMetaspaceIterator();
~ClassLoaderDataGraphMetaspaceIterator();
bool repeat() { return _data != NULL; }
ClassLoaderMetaspace* get_next();
};
#endif // SHARE_CLASSFILE_CLASSLOADERDATAGRAPH_HPP
@@ -26,6 +26,7 @@
#include "classfile/classLoaderData.inline.hpp"
#include "classfile/classLoaderDataGraph.hpp"
#include "classfile/classLoaderStats.hpp"
#include "memory/classLoaderMetaspace.hpp"
#include "oops/objArrayKlass.hpp"
#include "oops/oop.inline.hpp"
#include "utilities/globalDefinitions.hpp"
@@ -80,15 +81,17 @@ void ClassLoaderStatsClosure::do_cld(ClassLoaderData* cld) {

ClassLoaderMetaspace* ms = cld->metaspace_or_null();
if (ms != NULL) {
size_t used_bytes, capacity_bytes;
ms->calculate_jfr_stats(&used_bytes, &capacity_bytes);
if(cld->has_class_mirror_holder()) {
cls->_hidden_chunk_sz += ms->allocated_chunks_bytes();
cls->_hidden_block_sz += ms->allocated_blocks_bytes();
cls->_hidden_chunk_sz += capacity_bytes;
cls->_hidden_block_sz += used_bytes;
} else {
cls->_chunk_sz = ms->allocated_chunks_bytes();
cls->_block_sz = ms->allocated_blocks_bytes();
cls->_chunk_sz = capacity_bytes;
cls->_block_sz = used_bytes;
}
_total_chunk_sz += ms->allocated_chunks_bytes();
_total_block_sz += ms->allocated_blocks_bytes();
_total_chunk_sz += capacity_bytes;
_total_block_sz += used_bytes;
}
}

@@ -1034,7 +1034,7 @@ void G1CollectedHeap::prepare_heap_for_mutators() {

// Delete metaspaces for unloaded class loaders and clean up loader_data graph
ClassLoaderDataGraph::purge(/*at_safepoint*/true);
MetaspaceUtils::verify_metrics();
DEBUG_ONLY(MetaspaceUtils::verify();)

// Prepare heap for normal collections.
assert(num_free_regions() == 0, "we should not have added any free regions");
@@ -1058,7 +1058,7 @@ void PSParallelCompact::post_compact()

// Delete metaspaces for unloaded class loaders and clean up loader_data graph
ClassLoaderDataGraph::purge(/*at_safepoint*/true);
MetaspaceUtils::verify_metrics();
DEBUG_ONLY(MetaspaceUtils::verify();)
Copy link
Member

@iklam iklam Sep 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it will be cleaner to declare MetaspaceUtils::verify() as

void verify() NOT_DEBUG_RETURN;

then you can omit the DEBUG_ONLY at every caller.

Copy link
Member Author

@tstuefe tstuefe Sep 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless you really want me to, or other Reviewers chime in, I'd rather leave it as it is. I prefer this style since it is clear at the callsites that this is only ASSERT code.


heap->prune_scavengable_nmethods();

@@ -37,7 +37,7 @@
#include "gc/shared/memAllocator.hpp"
#include "logging/log.hpp"
#include "logging/logStream.hpp"
#include "memory/metaspace.hpp"
#include "memory/classLoaderMetaspace.hpp"
#include "memory/resourceArea.hpp"
#include "memory/universe.hpp"
#include "oops/instanceMirrorKlass.hpp"
@@ -32,6 +32,7 @@
#include "gc/shared/genCollectedHeap.hpp"
#include "interpreter/oopMapCache.hpp"
#include "logging/log.hpp"
#include "memory/classLoaderMetaspace.hpp"
#include "memory/oopFactory.hpp"
#include "memory/universe.hpp"
#include "runtime/handles.inline.hpp"
@@ -28,6 +28,7 @@
#include "gc/shared/collectedHeap.hpp"
#include "gc/shared/genCollectedHeap.hpp"
#include "memory/heapInspection.hpp"
#include "memory/metaspace.hpp"
#include "prims/jvmtiExport.hpp"
#include "runtime/handles.hpp"
#include "runtime/jniHandles.hpp"
@@ -58,6 +58,7 @@
#include "gc/shared/workgroup.hpp"
#include "memory/filemap.hpp"
#include "memory/iterator.hpp"
#include "memory/metaspace/metaspaceSizesSnapshot.hpp"
#include "memory/metaspaceCounters.hpp"
#include "memory/resourceArea.hpp"
#include "memory/universe.hpp"
@@ -662,7 +663,7 @@ void GenCollectedHeap::do_collection(bool full,

// Delete metaspaces for unloaded class loaders and clean up loader_data graph
ClassLoaderDataGraph::purge(/*at_safepoint*/true);
MetaspaceUtils::verify_metrics();
DEBUG_ONLY(MetaspaceUtils::verify();)
// Resize the metaspace capacity after full collections
MetaspaceGC::compute_new_size();
update_full_collections_completed();
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2001, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2001, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -25,7 +25,6 @@
#include "precompiled.hpp"
#include "gc/shared/cardTableRS.hpp"
#include "gc/shared/generationSpec.hpp"
#include "memory/binaryTreeDictionary.hpp"
#include "memory/filemap.hpp"
#include "runtime/java.hpp"
#include "utilities/macros.hpp"
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2015, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -262,18 +262,6 @@ JVMFlag::Error GCPauseIntervalMillisConstraintFunc(uintx value, bool verbose) {
return JVMFlag::SUCCESS;
}

JVMFlag::Error InitialBootClassLoaderMetaspaceSizeConstraintFunc(size_t value, bool verbose) {
size_t aligned_max = align_down(max_uintx/2, Metaspace::reserve_alignment_words());
if (value > aligned_max) {
JVMFlag::printError(verbose,
"InitialBootClassLoaderMetaspaceSize (" SIZE_FORMAT ") must be "
"less than or equal to aligned maximum value (" SIZE_FORMAT ")\n",
value, aligned_max);
return JVMFlag::VIOLATES_CONSTRAINT;
}
return JVMFlag::SUCCESS;
}

// To avoid an overflow by 'align_up(value, alignment)'.
static JVMFlag::Error MaxSizeForAlignment(const char* name, size_t value, size_t alignment, bool verbose) {
size_t aligned_max = ((max_uintx - alignment) & ~(alignment-1));
@@ -56,7 +56,6 @@
\
f(uintx, MaxGCPauseMillisConstraintFunc) \
f(uintx, GCPauseIntervalMillisConstraintFunc) \
f(size_t, InitialBootClassLoaderMetaspaceSizeConstraintFunc) \
f(size_t, MinHeapSizeConstraintFunc) \
f(size_t, InitialHeapSizeConstraintFunc) \
f(size_t, MaxHeapSizeConstraintFunc) \
@@ -71,7 +71,7 @@
#include "gc/shenandoah/shenandoahJfrSupport.hpp"
#endif

#include "memory/metaspace.hpp"
#include "memory/classLoaderMetaspace.hpp"
#include "oops/compressedOops.inline.hpp"
#include "runtime/atomic.hpp"
#include "runtime/globals.hpp"
@@ -2410,7 +2410,7 @@ void ShenandoahHeap::stw_unload_classes(bool full_gc) {
}
// Resize and verify metaspace
MetaspaceGC::compute_new_size();
MetaspaceUtils::verify_metrics();
DEBUG_ONLY(MetaspaceUtils::verify();)
}

// Weak roots are either pre-evacuated (final mark) or updated (final updaterefs),
@@ -35,6 +35,7 @@
#include "gc/shenandoah/shenandoahPadding.hpp"
#include "gc/shenandoah/shenandoahSharedVariables.hpp"
#include "gc/shenandoah/shenandoahUnload.hpp"
#include "memory/metaspace.hpp"
#include "services/memoryManager.hpp"
#include "utilities/globalDefinitions.hpp"
#include "utilities/stack.hpp"