8164921: Memory leaked when instrumentation.retransformClasses() is called repeatedly
Summary: Return Metablocks smaller than dictionary's dark matter.
Reviewed-by: mgerdin, sspitsyn, dsamersoff
Contributed-by: jon.masamitsu@oracle.com, coleen.phillimore@oracle.com
--- a/hotspot/src/share/vm/classfile/classLoaderData.cpp Tue Oct 18 13:24:02 2016 +0200
+++ b/hotspot/src/share/vm/classfile/classLoaderData.cpp Tue Oct 18 08:39:43 2016 -0400
@@ -639,7 +639,6 @@
#undef CLD_DUMP_KLASSES
void ClassLoaderData::dump(outputStream * const out) {
- ResourceMark rm;
out->print("ClassLoaderData CLD: " PTR_FORMAT ", loader: " PTR_FORMAT ", loader_klass: " PTR_FORMAT " %s {",
p2i(this), p2i((void *)class_loader()),
p2i(class_loader() != NULL ? class_loader()->klass() : NULL), loader_name());
@@ -656,7 +655,6 @@
#ifdef CLD_DUMP_KLASSES
if (Verbose) {
- ResourceMark rm;
Klass* k = _klasses;
while (k != NULL) {
out->print_cr("klass " PTR_FORMAT ", %s, CT: %d, MUT: %d", k, k->name()->as_C_string(),
--- a/hotspot/src/share/vm/logging/logTag.hpp Tue Oct 18 13:24:02 2016 +0200
+++ b/hotspot/src/share/vm/logging/logTag.hpp Tue Oct 18 08:39:43 2016 -0400
@@ -40,6 +40,7 @@
LOG_TAG(attach) \
LOG_TAG(barrier) \
LOG_TAG(biasedlocking) \
+ LOG_TAG(blocks) \
LOG_TAG(bot) \
LOG_TAG(breakpoint) \
LOG_TAG(census) \
--- a/hotspot/src/share/vm/memory/metaspace.cpp Tue Oct 18 13:24:02 2016 +0200
+++ b/hotspot/src/share/vm/memory/metaspace.cpp Tue Oct 18 08:39:43 2016 -0400
@@ -249,10 +249,65 @@
void print_on(outputStream* st) const;
};
+class SmallBlocks : public CHeapObj<mtClass> {
+ const static uint _small_block_max_size = sizeof(TreeChunk<Metablock, FreeList<Metablock> >)/HeapWordSize;
+ const static uint _small_block_min_size = sizeof(Metablock)/HeapWordSize;
+
+ private:
+ FreeList<Metablock> _small_lists[_small_block_max_size - _small_block_min_size];
+
+ FreeList<Metablock>& list_at(size_t word_size) {
+ assert(word_size >= _small_block_min_size, "There are no metaspace objects less than %u words", _small_block_min_size);
+ return _small_lists[word_size - _small_block_min_size];
+ }
+
+ public:
+ SmallBlocks() {
+ for (uint i = _small_block_min_size; i < _small_block_max_size; i++) {
+ uint k = i - _small_block_min_size;
+ _small_lists[k].set_size(i);
+ }
+ }
+
+ size_t total_size() const {
+ size_t result = 0;
+ for (uint i = _small_block_min_size; i < _small_block_max_size; i++) {
+ uint k = i - _small_block_min_size;
+ result = result + _small_lists[k].count() * _small_lists[k].size();
+ }
+ return result;
+ }
+
+ static uint small_block_max_size() { return _small_block_max_size; }
+ static uint small_block_min_size() { return _small_block_min_size; }
+
+ MetaWord* get_block(size_t word_size) {
+ if (list_at(word_size).count() > 0) {
+ MetaWord* new_block = (MetaWord*) list_at(word_size).get_chunk_at_head();
+ return new_block;
+ } else {
+ return NULL;
+ }
+ }
+ void return_block(Metablock* free_chunk, size_t word_size) {
+ list_at(word_size).return_chunk_at_head(free_chunk, false);
+ assert(list_at(word_size).count() > 0, "Should have a chunk");
+ }
+
+ void print_on(outputStream* st) const {
+ st->print_cr("SmallBlocks:");
+ for (uint i = _small_block_min_size; i < _small_block_max_size; i++) {
+ uint k = i - _small_block_min_size;
+ st->print_cr("small_lists size " SIZE_FORMAT " count " SIZE_FORMAT, _small_lists[k].size(), _small_lists[k].count());
+ }
+ }
+};
+
// Used to manage the free list of Metablocks (a block corresponds
// to the allocation of a quantum of metadata).
-class BlockFreelist VALUE_OBJ_CLASS_SPEC {
+class BlockFreelist : public CHeapObj<mtClass> {
BlockTreeDictionary* const _dictionary;
+ SmallBlocks* _small_blocks;
// Only allocate and split from freelist if the size of the allocation
// is at least 1/4th the size of the available block.
@@ -260,6 +315,12 @@
// Accessors
BlockTreeDictionary* dictionary() const { return _dictionary; }
+ SmallBlocks* small_blocks() {
+ if (_small_blocks == NULL) {
+ _small_blocks = new SmallBlocks();
+ }
+ return _small_blocks;
+ }
public:
BlockFreelist();
@@ -269,8 +330,15 @@
MetaWord* get_block(size_t word_size);
void return_block(MetaWord* p, size_t word_size);
- size_t total_size() { return dictionary()->total_size(); }
-
+ size_t total_size() const {
+ size_t result = dictionary()->total_size();
+ if (_small_blocks != NULL) {
+ result = result + _small_blocks->total_size();
+ }
+ return result;
+ }
+
+ static size_t min_dictionary_size() { return TreeChunk<Metablock, FreeList<Metablock> >::min_size(); }
void print_on(outputStream* st) const;
};
@@ -629,7 +697,7 @@
// are assumed to be in chunks in use by the SpaceManager
// and all chunks in use by a SpaceManager are freed when
// the class loader using the SpaceManager is collected.
- BlockFreelist _block_freelists;
+ BlockFreelist* _block_freelists;
// protects virtualspace and chunk expansions
static const char* _expand_lock_name;
@@ -643,9 +711,7 @@
_chunks_in_use[index] = v;
}
- BlockFreelist* block_freelists() const {
- return (BlockFreelist*) &_block_freelists;
- }
+ BlockFreelist* block_freelists() const { return _block_freelists; }
Metaspace::MetadataType mdtype() { return _mdtype; }
@@ -763,7 +829,9 @@
void verify_allocated_blocks_words();
#endif
- size_t get_raw_word_size(size_t word_size) {
+ // This adjusts the size given to be greater than the minimum allocation size in
+ // words for data in metaspace. Esentially the minimum size is currently 3 words.
+ size_t get_allocation_word_size(size_t word_size) {
size_t byte_size = word_size * BytesPerWord;
size_t raw_bytes_size = MAX2(byte_size, sizeof(Metablock));
@@ -807,20 +875,45 @@
// BlockFreelist methods
-BlockFreelist::BlockFreelist() : _dictionary(new BlockTreeDictionary()) {}
+BlockFreelist::BlockFreelist() : _dictionary(new BlockTreeDictionary()), _small_blocks(NULL) {}
BlockFreelist::~BlockFreelist() {
delete _dictionary;
+ if (_small_blocks != NULL) {
+ delete _small_blocks;
+ }
}
void BlockFreelist::return_block(MetaWord* p, size_t word_size) {
+ assert(word_size >= SmallBlocks::small_block_min_size(), "never return dark matter");
+
Metablock* free_chunk = ::new (p) Metablock(word_size);
+ if (word_size < SmallBlocks::small_block_max_size()) {
+ small_blocks()->return_block(free_chunk, word_size);
+ } else {
dictionary()->return_chunk(free_chunk);
}
+ log_trace(gc, metaspace, freelist, blocks)("returning block at " INTPTR_FORMAT " size = "
+ SIZE_FORMAT, p2i(free_chunk), word_size);
+}
MetaWord* BlockFreelist::get_block(size_t word_size) {
- if (word_size < TreeChunk<Metablock, FreeList<Metablock> >::min_size()) {
- // Dark matter. Too small for dictionary.
+ assert(word_size >= SmallBlocks::small_block_min_size(), "never get dark matter");
+
+ // Try small_blocks first.
+ if (word_size < SmallBlocks::small_block_max_size()) {
+ // Don't create small_blocks() until needed. small_blocks() allocates the small block list for
+ // this space manager.
+ MetaWord* new_block = (MetaWord*) small_blocks()->get_block(word_size);
+ if (new_block != NULL) {
+ log_trace(gc, metaspace, freelist, blocks)("getting block at " INTPTR_FORMAT " size = " SIZE_FORMAT,
+ p2i(new_block), word_size);
+ return new_block;
+ }
+ }
+
+ if (word_size < BlockFreelist::min_dictionary_size()) {
+ // If allocation in small blocks fails, this is Dark Matter. Too small for dictionary.
return NULL;
}
@@ -839,15 +932,20 @@
MetaWord* new_block = (MetaWord*)free_block;
assert(block_size >= word_size, "Incorrect size of block from freelist");
const size_t unused = block_size - word_size;
- if (unused >= TreeChunk<Metablock, FreeList<Metablock> >::min_size()) {
+ if (unused >= SmallBlocks::small_block_min_size()) {
return_block(new_block + word_size, unused);
}
+ log_trace(gc, metaspace, freelist, blocks)("getting block at " INTPTR_FORMAT " size = " SIZE_FORMAT,
+ p2i(new_block), word_size);
return new_block;
}
void BlockFreelist::print_on(outputStream* st) const {
dictionary()->print_free_lists(st);
+ if (_small_blocks != NULL) {
+ _small_blocks->print_on(st);
+ }
}
// VirtualSpaceNode methods
@@ -2075,6 +2173,7 @@
_allocated_blocks_words(0),
_allocated_chunks_words(0),
_allocated_chunks_count(0),
+ _block_freelists(NULL),
_lock(lock)
{
initialize();
@@ -2164,8 +2263,10 @@
log.trace("~SpaceManager(): " PTR_FORMAT, p2i(this));
ResourceMark rm;
locked_print_chunks_in_use_on(log.trace_stream());
+ if (block_freelists() != NULL) {
block_freelists()->print_on(log.trace_stream());
}
+ }
// Have to update before the chunks_in_use lists are emptied
// below.
@@ -2215,6 +2316,10 @@
}
log.trace("updated dictionary count " SIZE_FORMAT " %s", chunk_manager()->humongous_dictionary()->total_count(), chunk_size_name(HumongousIndex));
chunk_manager()->slow_locked_verify();
+
+ if (_block_freelists != NULL) {
+ delete _block_freelists;
+ }
}
const char* SpaceManager::chunk_size_name(ChunkIndex index) const {
@@ -2253,10 +2358,12 @@
void SpaceManager::deallocate(MetaWord* p, size_t word_size) {
assert_lock_strong(_lock);
- size_t raw_word_size = get_raw_word_size(word_size);
- size_t min_size = TreeChunk<Metablock, FreeList<Metablock> >::min_size();
- assert(raw_word_size >= min_size,
- "Should not deallocate dark matter " SIZE_FORMAT "<" SIZE_FORMAT, word_size, min_size);
+ // Allocations and deallocations are in raw_word_size
+ size_t raw_word_size = get_allocation_word_size(word_size);
+ // Lazily create a block_freelist
+ if (block_freelists() == NULL) {
+ _block_freelists = new BlockFreelist();
+ }
block_freelists()->return_block(p, raw_word_size);
}
@@ -2312,8 +2419,9 @@
void SpaceManager::retire_current_chunk() {
if (current_chunk() != NULL) {
size_t remaining_words = current_chunk()->free_word_size();
- if (remaining_words >= TreeChunk<Metablock, FreeList<Metablock> >::min_size()) {
- block_freelists()->return_block(current_chunk()->allocate(remaining_words), remaining_words);
+ if (remaining_words >= BlockFreelist::min_dictionary_size()) {
+ MetaWord* ptr = current_chunk()->allocate(remaining_words);
+ deallocate(ptr, remaining_words);
inc_used_metrics(remaining_words);
}
}
@@ -2350,7 +2458,7 @@
* will be made to allocate a small chunk.
*/
MetaWord* SpaceManager::get_small_chunk_and_allocate(size_t word_size) {
- size_t raw_word_size = get_raw_word_size(word_size);
+ size_t raw_word_size = get_allocation_word_size(word_size);
if (raw_word_size + Metachunk::overhead() > small_chunk_size()) {
return NULL;
@@ -2380,8 +2488,7 @@
MetaWord* SpaceManager::allocate(size_t word_size) {
MutexLockerEx cl(lock(), Mutex::_no_safepoint_check_flag);
-
- size_t raw_word_size = get_raw_word_size(word_size);
+ size_t raw_word_size = get_allocation_word_size(word_size);
BlockFreelist* fl = block_freelists();
MetaWord* p = NULL;
// Allocation from the dictionary is expensive in the sense that
@@ -2389,7 +2496,7 @@
// from the dictionary until it starts to get fat. Is this
// a reasonable policy? Maybe an skinny dictionary is fast enough
// for allocations. Do some profiling. JJJ
- if (fl->total_size() > allocation_from_dictionary_limit) {
+ if (fl != NULL && fl->total_size() > allocation_from_dictionary_limit) {
p = fl->get_block(raw_word_size);
}
if (p == NULL) {
@@ -2441,7 +2548,7 @@
// If there are blocks in the dictionary, then
// verification of chunks does not work since
// being in the dictionary alters a chunk.
- if (block_freelists()->total_size() == 0) {
+ if (block_freelists() != NULL && block_freelists()->total_size() == 0) {
for (ChunkIndex i = ZeroIndex; i < NumberOfInUseLists; i = next_chunk_index(i)) {
Metachunk* curr = chunks_in_use(i);
while (curr != NULL) {
@@ -2499,7 +2606,7 @@
}
if (log_is_enabled(Trace, gc, metaspace, freelist)) {
- block_freelists()->print_on(out);
+ if (block_freelists() != NULL) block_freelists()->print_on(out);
}
size_t free = current_chunk() == NULL ? 0 : current_chunk()->free_word_size();
@@ -3410,18 +3517,11 @@
|| Thread::current()->is_VM_thread(), "should be the VM thread");
if (DumpSharedSpaces && PrintSharedSpaces) {
- record_deallocation(ptr, vsm()->get_raw_word_size(word_size));
+ record_deallocation(ptr, vsm()->get_allocation_word_size(word_size));
}
MutexLockerEx ml(vsm()->lock(), Mutex::_no_safepoint_check_flag);
- if (word_size < TreeChunk<Metablock, FreeList<Metablock> >::min_size()) {
- // Dark matter. Too small for dictionary.
-#ifdef ASSERT
- Copy::fill_to_words((HeapWord*)ptr, word_size, 0xf5f5f5f5);
-#endif
- return;
- }
if (is_class && using_class_space()) {
class_vsm()->deallocate(ptr, word_size);
} else {
@@ -3451,7 +3551,7 @@
report_out_of_shared_space(read_only ? SharedReadOnly : SharedReadWrite);
}
if (PrintSharedSpaces) {
- space->record_allocation(result, type, space->vsm()->get_raw_word_size(word_size));
+ space->record_allocation(result, type, space->vsm()->get_allocation_word_size(word_size));
}
// Zero initialize.
@@ -3509,10 +3609,11 @@
// If result is still null, we are out of memory.
Log(gc, metaspace, freelist) log;
- if (log.is_trace()) {
- log.trace("Metaspace allocation failed for size " SIZE_FORMAT, word_size);
+ if (log.is_info()) {
+ log.info("Metaspace (%s) allocation failed for size " SIZE_FORMAT,
+ is_class_space_allocation(mdtype) ? "class" : "data", word_size);
ResourceMark rm;
- outputStream* out = log.trace_stream();
+ outputStream* out = log.info_stream();
if (loader_data->metaspace_or_null() != NULL) {
loader_data->dump(out);
}
--- a/hotspot/src/share/vm/oops/constMethod.cpp Tue Oct 18 13:24:02 2016 +0200
+++ b/hotspot/src/share/vm/oops/constMethod.cpp Tue Oct 18 08:39:43 2016 -0400
@@ -368,23 +368,36 @@
return (AnnotationArray**)constMethod_end() - offset;
}
+Array<u1>* copy_annotations(ClassLoaderData* loader_data, AnnotationArray* from, TRAPS) {
+ int length = from->length();
+ Array<u1>* a = MetadataFactory::new_array<u1>(loader_data, length, 0, CHECK_NULL);
+ memcpy((void*)a->adr_at(0), (void*)from->adr_at(0), length);
+ return a;
+}
+
// copy annotations from 'cm' to 'this'
-void ConstMethod::copy_annotations_from(ConstMethod* cm) {
+// Must make copy because these are deallocated with their constMethod, if redefined.
+void ConstMethod::copy_annotations_from(ClassLoaderData* loader_data, ConstMethod* cm, TRAPS) {
+ Array<u1>* a;
if (cm->has_method_annotations()) {
assert(has_method_annotations(), "should be allocated already");
- set_method_annotations(cm->method_annotations());
+ a = copy_annotations(loader_data, cm->method_annotations(), CHECK);
+ set_method_annotations(a);
}
if (cm->has_parameter_annotations()) {
assert(has_parameter_annotations(), "should be allocated already");
- set_parameter_annotations(cm->parameter_annotations());
+ a = copy_annotations(loader_data, cm->parameter_annotations(), CHECK);
+ set_parameter_annotations(a);
}
if (cm->has_type_annotations()) {
assert(has_type_annotations(), "should be allocated already");
- set_type_annotations(cm->type_annotations());
+ a = copy_annotations(loader_data, cm->type_annotations(), CHECK);
+ set_type_annotations(a);
}
if (cm->has_default_annotations()) {
assert(has_default_annotations(), "should be allocated already");
- set_default_annotations(cm->default_annotations());
+ a = copy_annotations(loader_data, cm->default_annotations(), CHECK);
+ set_default_annotations(a);
}
}
--- a/hotspot/src/share/vm/oops/constMethod.hpp Tue Oct 18 13:24:02 2016 +0200
+++ b/hotspot/src/share/vm/oops/constMethod.hpp Tue Oct 18 08:39:43 2016 -0400
@@ -469,7 +469,7 @@
}
// Copy annotations from other ConstMethod
- void copy_annotations_from(ConstMethod* cm);
+ void copy_annotations_from(ClassLoaderData* loader_data, ConstMethod* cm, TRAPS);
// byte codes
void set_code(address code) {
--- a/hotspot/src/share/vm/oops/method.cpp Tue Oct 18 13:24:02 2016 +0200
+++ b/hotspot/src/share/vm/oops/method.cpp Tue Oct 18 08:39:43 2016 -0400
@@ -1380,7 +1380,7 @@
}
// copy annotations over to new method
- newcm->copy_annotations_from(cm);
+ newcm->copy_annotations_from(loader_data, cm, CHECK_NULL);
return newm;
}
--- a/hotspot/test/TEST.groups Tue Oct 18 13:24:02 2016 +0200
+++ b/hotspot/test/TEST.groups Tue Oct 18 08:39:43 2016 -0400
@@ -370,6 +370,7 @@
-runtime/modules/ModuleStress/ModuleStressGC.java \
-runtime/NMT \
-runtime/RedefineObject/TestRedefineObject.java \
+ -runtime/RedefineTests/RedefineLeak.java \
-runtime/RedefineTests/RedefinePreviousVersions.java \
-runtime/RedefineTests/RedefineRunningMethods.java \
-runtime/RedefineTests/RedefineRunningMethodsWithBacktrace.java \
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/runtime/RedefineTests/RedefineLeak.java Tue Oct 18 08:39:43 2016 -0400
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2016, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @library /test/lib
+ * @summary Test that redefinition reuses metaspace blocks that are freed
+ * @modules java.base/jdk.internal.misc
+ * @modules java.instrument
+ * jdk.jartool/sun.tools.jar
+ * @run main RedefineLeak buildagent
+ * @run main/othervm/timeout=6000 RedefineLeak runtest
+ */
+
+import java.io.FileNotFoundException;
+import java.io.PrintWriter;
+import java.lang.RuntimeException;
+import java.lang.instrument.ClassFileTransformer;
+import java.lang.instrument.Instrumentation;
+import java.security.ProtectionDomain;
+import java.lang.instrument.IllegalClassFormatException;
+import jdk.test.lib.process.ProcessTools;
+import jdk.test.lib.process.OutputAnalyzer;
+
+public class RedefineLeak {
+ static class Tester {}
+
+ static class LoggingTransformer implements ClassFileTransformer {
+ static int transformCount = 0;
+
+ public LoggingTransformer() {}
+
+ public byte[] transform(ClassLoader loader, String className, Class classBeingRedefined,
+ ProtectionDomain protectionDomain, byte[] classfileBuffer) throws IllegalClassFormatException {
+
+ transformCount++;
+ if (transformCount % 1000 == 0) System.out.println("transformCount:" + transformCount);
+ return null;
+ }
+ }
+
+ public static void premain(String agentArgs, Instrumentation inst) throws Exception {
+ LoggingTransformer t = new LoggingTransformer();
+ inst.addTransformer(t, true);
+ {
+ Class demoClass = Class.forName("RedefineLeak$Tester");
+
+ for (int i = 0; i < 10000; i++) {
+ inst.retransformClasses(demoClass);
+ }
+ }
+ System.gc();
+ }
+ private static void buildAgent() {
+ try {
+ ClassFileInstaller.main("RedefineLeak");
+ } catch (Exception e) {
+ throw new RuntimeException("Could not write agent classfile", e);
+ }
+
+ try {
+ PrintWriter pw = new PrintWriter("MANIFEST.MF");
+ pw.println("Premain-Class: RedefineLeak");
+ pw.println("Agent-Class: RedefineLeak");
+ pw.println("Can-Redefine-Classes: true");
+ pw.println("Can-Retransform-Classes: true");
+ pw.close();
+ } catch (FileNotFoundException e) {
+ throw new RuntimeException("Could not write manifest file for the agent", e);
+ }
+
+ sun.tools.jar.Main jarTool = new sun.tools.jar.Main(System.out, System.err, "jar");
+ if (!jarTool.run(new String[] { "-cmf", "MANIFEST.MF", "redefineagent.jar", "RedefineLeak.class" })) {
+ throw new RuntimeException("Could not write the agent jar file");
+ }
+ }
+ public static void main(String argv[]) throws Exception {
+ if (argv.length == 1 && argv[0].equals("buildagent")) {
+ buildAgent();
+ return;
+ }
+ if (argv.length == 1 && argv[0].equals("runtest")) {
+ // run outside of jtreg to not OOM on jtreg classes that are loaded after metaspace is full
+ String[] javaArgs1 = { "-XX:MetaspaceSize=12m", "-XX:MaxMetaspaceSize=12m",
+ "-javaagent:redefineagent.jar", "RedefineLeak"};
+ ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(javaArgs1);
+
+ OutputAnalyzer output = new OutputAnalyzer(pb.start());
+ output.shouldContain("transformCount:10000");
+ }
+ }
+}