# HG changeset patch # User coleenp # Date 1476794383 14400 # Node ID f1658e76a682d4807abfcda8d86cfc712b2d0313 # Parent d2a206359a7b4e8681137d35e909cc461078deda 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 diff -r d2a206359a7b -r f1658e76a682 hotspot/src/share/vm/classfile/classLoaderData.cpp --- 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(), diff -r d2a206359a7b -r f1658e76a682 hotspot/src/share/vm/logging/logTag.hpp --- 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) \ diff -r d2a206359a7b -r f1658e76a682 hotspot/src/share/vm/memory/metaspace.cpp --- 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 { + const static uint _small_block_max_size = sizeof(TreeChunk >)/HeapWordSize; + const static uint _small_block_min_size = sizeof(Metablock)/HeapWordSize; + + private: + FreeList _small_lists[_small_block_max_size - _small_block_min_size]; + + FreeList& 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 { 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 >::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 >::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 >::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 >::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 >::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 >::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); } diff -r d2a206359a7b -r f1658e76a682 hotspot/src/share/vm/oops/constMethod.cpp --- 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* copy_annotations(ClassLoaderData* loader_data, AnnotationArray* from, TRAPS) { + int length = from->length(); + Array* a = MetadataFactory::new_array(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* 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); } } diff -r d2a206359a7b -r f1658e76a682 hotspot/src/share/vm/oops/constMethod.hpp --- 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) { diff -r d2a206359a7b -r f1658e76a682 hotspot/src/share/vm/oops/method.cpp --- 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; } diff -r d2a206359a7b -r f1658e76a682 hotspot/test/TEST.groups --- 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 \ diff -r d2a206359a7b -r f1658e76a682 hotspot/test/runtime/RedefineTests/RedefineLeak.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"); + } + } +}