# HG changeset patch # User ehelin # Date 1411568964 -7200 # Wed Sep 24 16:29:24 2014 +0200 # Node ID 39b8dfa5571426df8f8d236e0a2158e0f7472c8f # Parent d527d89ab0fdebbd905f05e6420e31ac7c3b52aa 8049599: MetaspaceGC::_capacity_until_GC can overflow (01) diff --git a/src/share/vm/memory/metaspace.cpp b/src/share/vm/memory/metaspace.cpp --- a/src/share/vm/memory/metaspace.cpp +++ b/src/share/vm/memory/metaspace.cpp @@ -1410,42 +1410,41 @@ size_t MetaspaceGC::delta_capacity_until } size_t MetaspaceGC::capacity_until_GC() { size_t value = (size_t)OrderAccess::load_ptr_acquire(&_capacity_until_GC); assert(value >= MetaspaceSize, "Not initialied properly?"); return value; } -size_t MetaspaceGC::inc_capacity_until_GC(size_t v) { +bool MetaspaceGC::inc_capacity_until_GC(size_t v, size_t* new_cap_until_GC, size_t* old_cap_until_GC) { assert_is_size_aligned(v, Metaspace::commit_alignment()); size_t capacity_until_GC = (size_t) _capacity_until_GC; size_t new_value = capacity_until_GC + v; if (new_value < capacity_until_GC) { - // the addition wrapped around, set new_value to max value - new_value = max_uintx; + // the addition wrapped around, set new_value to aligned max value + new_value = align_size_down(max_uintx, Metaspace::commit_alignment()); } - bool cas_succeeded = false; - size_t num_retries = 5; // just to limit the number of CAS attempts - for (size_t i = 0; i < num_retries; i++) { - intptr_t cmp = (intptr_t) capacity_until_GC; - intptr_t swap = (intptr_t) new_value; - intptr_t prev = Atomic::cmpxchg_ptr(swap, &_capacity_until_GC, cmp); - - if (prev == cmp) { - cas_succeeded = true; - break; - } + intptr_t expected = (intptr_t) capacity_until_GC; + intptr_t actual = Atomic::cmpxchg((intptr_t) new_value, &_capacity_until_GC, expected); + + if (expected != actual) { + return false; } - // if the CAS did not succeed, just return the current value - return cas_succeeded ? new_value : _capacity_until_GC; + if (new_cap_until_GC != NULL) { + *new_cap_until_GC = new_value; + } + if (old_cap_until_GC != NULL) { + *old_cap_until_GC = capacity_until_GC; + } + return true; } size_t MetaspaceGC::dec_capacity_until_GC(size_t v) { assert_is_size_aligned(v, Metaspace::commit_alignment()); return (size_t)Atomic::add_ptr(-(intptr_t)v, &_capacity_until_GC); } @@ -1535,17 +1534,20 @@ void MetaspaceGC::compute_new_size() { size_t shrink_bytes = 0; if (capacity_until_GC < minimum_desired_capacity) { // If we have less capacity below the metaspace HWM, then // increment the HWM. size_t expand_bytes = minimum_desired_capacity - capacity_until_GC; expand_bytes = align_size_up(expand_bytes, Metaspace::commit_alignment()); // Don't expand unless it's significant if (expand_bytes >= MinMetaspaceExpansion) { - size_t new_capacity_until_GC = MetaspaceGC::inc_capacity_until_GC(expand_bytes); + size_t new_capacity_until_GC = 0; + bool succeeded = MetaspaceGC::inc_capacity_until_GC(expand_bytes, &new_capacity_until_GC); + assert(succeeded, "Should always succesfully increment HWM when at safepoint"); + Metaspace::tracer()->report_gc_threshold(capacity_until_GC, new_capacity_until_GC, MetaspaceGCThresholdUpdater::ComputeNewSize); if (PrintGCDetails && Verbose) { gclog_or_tty->print_cr(" expanding:" " minimum_desired_capacity: %6.1fKB" " expand_bytes: %6.1fKB" " MinMetaspaceExpansion: %6.1fKB" @@ -3338,29 +3340,38 @@ MetaWord* Metaspace::allocate(size_t wor return vsm()->allocate(word_size); } } MetaWord* Metaspace::expand_and_allocate(size_t word_size, MetadataType mdtype) { size_t delta_bytes = MetaspaceGC::delta_capacity_until_GC(word_size * BytesPerWord); assert(delta_bytes > 0, "Must be"); - size_t after_inc = MetaspaceGC::inc_capacity_until_GC(delta_bytes); - - // capacity_until_GC might be updated concurrently, must calculate previous value. - size_t before_inc = after_inc - delta_bytes; - - tracer()->report_gc_threshold(before_inc, after_inc, - MetaspaceGCThresholdUpdater::ExpandAndAllocate); - if (PrintGCDetails && Verbose) { - gclog_or_tty->print_cr("Increase capacity to GC from " SIZE_FORMAT - " to " SIZE_FORMAT, before_inc, after_inc); + size_t before_inc = 0; + size_t after_inc = 0; + MetaWord* res = NULL; + bool inc_succeeded = false; + + while (!inc_succeeded && res == NULL) { + inc_succeeded = MetaspaceGC::inc_capacity_until_GC(delta_bytes, + &after_inc, + &before_inc); + res = allocate(word_size, mdtype); } - return allocate(word_size, mdtype); + if (inc_succeeded) { + tracer()->report_gc_threshold(before_inc, after_inc, + MetaspaceGCThresholdUpdater::ExpandAndAllocate); + if (PrintGCDetails && Verbose) { + gclog_or_tty->print_cr("Increase capacity to GC from " SIZE_FORMAT + " to " SIZE_FORMAT, before_inc, after_inc); + } + } + + return res; } // Space allocated in the Metaspace. This may // be across several metadata virtual spaces. char* Metaspace::bottom() const { assert(DumpSharedSpaces, "only useful and valid for dumping shared spaces"); return (char*)vsm()->current_chunk()->bottom(); } diff --git a/src/share/vm/memory/metaspace.hpp b/src/share/vm/memory/metaspace.hpp --- a/src/share/vm/memory/metaspace.hpp +++ b/src/share/vm/memory/metaspace.hpp @@ -402,17 +402,19 @@ class MetaspaceGC : AllStatic { void set_shrink_factor(uint v) { _shrink_factor = v; } public: static void initialize(); static void post_initialize(); static size_t capacity_until_GC(); - static size_t inc_capacity_until_GC(size_t v); + static bool inc_capacity_until_GC(size_t v, + size_t* new_cap_until_GC = NULL, + size_t* old_cap_until_GC = NULL); static size_t dec_capacity_until_GC(size_t v); static bool should_concurrent_collect() { return _should_concurrent_collect; } static void set_should_concurrent_collect(bool v) { _should_concurrent_collect = v; } // The amount to increase the high-water-mark (_capacity_until_GC) diff --git a/src/share/vm/prims/whitebox.cpp b/src/share/vm/prims/whitebox.cpp --- a/src/share/vm/prims/whitebox.cpp +++ b/src/share/vm/prims/whitebox.cpp @@ -821,22 +821,30 @@ WB_ENTRY(void, WB_FreeMetaspace(JNIEnv* WB_END WB_ENTRY(jlong, WB_IncMetaspaceCapacityUntilGC(JNIEnv* env, jobject wb, jlong size)) if (size < 0) { THROW_MSG_0(vmSymbols::java_lang_IllegalArgumentException(), err_msg("WB_IncMetaspaceCapacityUntilGC: size is negative: " JLONG_FORMAT, size)); } - if (size > (jlong) ((size_t) -1)) { + jlong max_size_t = (jlong) ((size_t) -1); + if (size > max_size_t) { THROW_MSG_0(vmSymbols::java_lang_IllegalArgumentException(), err_msg("WB_IncMetaspaceCapacityUntilGC: size does not fit in size_t: " JLONG_FORMAT, size)); } - size_t new_val = MetaspaceGC::inc_capacity_until_GC(align_size_down((size_t) size, Metaspace::commit_alignment())); + size_t new_val = 0; + size_t aligned_increment = align_size_down((size_t) size, Metaspace::commit_alignment()); + bool success = MetaspaceGC::inc_capacity_until_GC(aligned_increment, &new_val); + if (!success) { + THROW_MSG_0(vmSymbols::java_lang_IllegalStateException(), + "WB_IncMetaspaceCapacityUntilGC: could not increase capacity until GC " + "due to contention with another thread"); + } return (jlong) new_val; WB_END WB_ENTRY(jlong, WB_MetaspaceCapacityUntilGC(JNIEnv* env, jobject wb)) return (jlong) MetaspaceGC::capacity_until_GC(); WB_END //Some convenience methods to deal with objects from java diff --git a/test/gc/metaspace/TestCapacityUntilGCWrapAround.java b/test/gc/metaspace/TestCapacityUntilGCWrapAround.java --- a/test/gc/metaspace/TestCapacityUntilGCWrapAround.java +++ b/test/gc/metaspace/TestCapacityUntilGCWrapAround.java @@ -45,14 +45,12 @@ public class TestCapacityUntilGCWrapArou public static void main(String[] args) { if (Platform.is32bit()) { WhiteBox wb = WhiteBox.getWhiteBox(); long before = wb.metaspaceCapacityUntilGC(); long after = wb.incMetaspaceCapacityUntilGC(MAX_UINT); Asserts.assertGTE(after, before, - String.format("Increasing with MAX_UINT should not cause wrap around: %d < %d", after, before)); - Asserts.assertEQ(after, MAX_UINT, - String.format("Increasing with MAX_UINT should set Metaspace::_capacityUntilGC to max_uint, not: %d", after)); + "Increasing with MAX_UINT should not cause wrap around: " + after + " < " + before); } } }