7121373: Clean up CollectedHeap::is_in
Summary: Fixed G1CollectedHeap::is_in, added tests, cleaned up comments and made Space::is_in pure virtual.
Reviewed-by: brutisso, tonyp, jcoomes
--- a/hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.hpp Fri Dec 09 19:28:34 2011 -0800
+++ b/hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/compactibleFreeListSpace.hpp Wed Dec 14 12:15:26 2011 +0100
@@ -336,12 +336,6 @@
unallocated_block() : end());
}
- // This is needed because the default implementation uses block_start()
- // which can;t be used at certain times (for example phase 3 of mark-sweep).
- // A better fix is to change the assertions in phase 3 of mark-sweep to
- // use is_in_reserved(), but that is deferred since the is_in() assertions
- // are buried through several layers of callers and are used elsewhere
- // as well.
bool is_in(const void* p) const {
return used_region().contains(p);
}
--- a/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp Fri Dec 09 19:28:34 2011 -0800
+++ b/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp Wed Dec 14 12:15:26 2011 +0100
@@ -2411,8 +2411,11 @@
}
bool G1CollectedHeap::is_in(const void* p) const {
- HeapRegion* hr = _hrs.addr_to_region((HeapWord*) p);
- if (hr != NULL) {
+ if (_g1_committed.contains(p)) {
+ // Given that we know that p is in the committed space,
+ // heap_region_containing_raw() should successfully
+ // return the containing region.
+ HeapRegion* hr = heap_region_containing_raw(p);
return hr->is_in(p);
} else {
return _perm_gen->as_gen()->is_in(p);
--- a/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp Fri Dec 09 19:28:34 2011 -0800
+++ b/hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp Wed Dec 14 12:15:26 2011 +0100
@@ -1196,7 +1196,7 @@
HumongousRegionSet* humongous_proxy_set,
bool par);
- // Returns "TRUE" iff "p" points into the allocated area of the heap.
+ // Returns "TRUE" iff "p" points into the committed areas of the heap.
virtual bool is_in(const void* p) const;
// Return "TRUE" iff the given object address is within the collection
--- a/hotspot/src/share/vm/gc_interface/collectedHeap.cpp Fri Dec 09 19:28:34 2011 -0800
+++ b/hotspot/src/share/vm/gc_interface/collectedHeap.cpp Wed Dec 14 12:15:26 2011 +0100
@@ -471,3 +471,26 @@
return mirror;
}
+
+/////////////// Unit tests ///////////////
+
+#ifndef PRODUCT
+void CollectedHeap::test_is_in() {
+ CollectedHeap* heap = Universe::heap();
+
+ // Test that NULL is not in the heap.
+ assert(!heap->is_in(NULL), "NULL is unexpectedly in the heap");
+
+ // Test that a pointer to before the heap start is reported as outside the heap.
+ assert(heap->_reserved.start() >= (void*)MinObjAlignment, "sanity");
+ void* before_heap = (void*)((intptr_t)heap->_reserved.start() - MinObjAlignment);
+ assert(!heap->is_in(before_heap),
+ err_msg("before_heap: " PTR_FORMAT " is unexpectedly in the heap", before_heap));
+
+ // Test that a pointer to after the heap end is reported as outside the heap.
+ assert(heap->_reserved.end() <= (void*)(uintptr_t(-1) - (uint)MinObjAlignment), "sanity");
+ void* after_heap = (void*)((intptr_t)heap->_reserved.end() + MinObjAlignment);
+ assert(!heap->is_in(after_heap),
+ err_msg("after_heap: " PTR_FORMAT " is unexpectedly in the heap", after_heap));
+}
+#endif
--- a/hotspot/src/share/vm/gc_interface/collectedHeap.hpp Fri Dec 09 19:28:34 2011 -0800
+++ b/hotspot/src/share/vm/gc_interface/collectedHeap.hpp Wed Dec 14 12:15:26 2011 +0100
@@ -217,8 +217,8 @@
return p == NULL || is_in_reserved(p);
}
- // Returns "TRUE" if "p" points to the head of an allocated object in the
- // heap. Since this method can be expensive in general, we restrict its
+ // Returns "TRUE" iff "p" points into the committed areas of the heap.
+ // Since this method can be expensive in general, we restrict its
// use to assertion checking only.
virtual bool is_in(const void* p) const = 0;
@@ -648,6 +648,10 @@
// reduce the occurrence of ParallelGCThreads to uses where the
// actual number may be germane.
static bool use_parallel_gc_threads() { return ParallelGCThreads > 0; }
+
+ /////////////// Unit tests ///////////////
+
+ NOT_PRODUCT(static void test_is_in();)
};
// Class to set and reset the GC cause for a CollectedHeap.
--- a/hotspot/src/share/vm/memory/genCollectedHeap.cpp Fri Dec 09 19:28:34 2011 -0800
+++ b/hotspot/src/share/vm/memory/genCollectedHeap.cpp Wed Dec 14 12:15:26 2011 +0100
@@ -957,7 +957,7 @@
return result;
}
-// Returns "TRUE" iff "p" points into the allocated area of the heap.
+// Returns "TRUE" iff "p" points into the committed areas of the heap.
bool GenCollectedHeap::is_in(const void* p) const {
#ifndef ASSERT
guarantee(VerifyBeforeGC ||
--- a/hotspot/src/share/vm/memory/genCollectedHeap.hpp Fri Dec 09 19:28:34 2011 -0800
+++ b/hotspot/src/share/vm/memory/genCollectedHeap.hpp Wed Dec 14 12:15:26 2011 +0100
@@ -198,7 +198,7 @@
// Mostly used for testing purposes. Caller does not hold the Heap_lock on entry.
void collect(GCCause::Cause cause, int max_level);
- // Returns "TRUE" iff "p" points into the allocated area of the heap.
+ // Returns "TRUE" iff "p" points into the committed areas of the heap.
// The methods is_in(), is_in_closed_subset() and is_in_youngest() may
// be expensive to compute in general, so, to prevent
// their inadvertent use in product jvm's, we restrict their use to
--- a/hotspot/src/share/vm/memory/generation.hpp Fri Dec 09 19:28:34 2011 -0800
+++ b/hotspot/src/share/vm/memory/generation.hpp Wed Dec 14 12:15:26 2011 +0100
@@ -220,7 +220,7 @@
MemRegion prev_used_region() const { return _prev_used_region; }
virtual void save_used_region() { _prev_used_region = used_region(); }
- // Returns "TRUE" iff "p" points into an allocated object in the generation.
+ // Returns "TRUE" iff "p" points into the committed areas in the generation.
// For some kinds of generations, this may be an expensive operation.
// To avoid performance problems stemming from its inadvertent use in
// product jvm's, we restrict its use to assertion checking or
--- a/hotspot/src/share/vm/memory/space.cpp Fri Dec 09 19:28:34 2011 -0800
+++ b/hotspot/src/share/vm/memory/space.cpp Wed Dec 14 12:15:26 2011 +0100
@@ -304,11 +304,6 @@
CompactibleSpace::clear(mangle_space);
}
-bool Space::is_in(const void* p) const {
- HeapWord* b = block_start_const(p);
- return b != NULL && block_is_obj(b);
-}
-
bool ContiguousSpace::is_in(const void* p) const {
return _bottom <= p && p < _top;
}
--- a/hotspot/src/share/vm/memory/space.hpp Fri Dec 09 19:28:34 2011 -0800
+++ b/hotspot/src/share/vm/memory/space.hpp Wed Dec 14 12:15:26 2011 +0100
@@ -187,7 +187,7 @@
// expensive operation. To prevent performance problems
// on account of its inadvertent use in product jvm's,
// we restrict its use to assertion checks only.
- virtual bool is_in(const void* p) const;
+ virtual bool is_in(const void* p) const = 0;
// Returns true iff the given reserved memory of the space contains the
// given address.
--- a/hotspot/src/share/vm/oops/arrayOop.cpp Fri Dec 09 19:28:34 2011 -0800
+++ b/hotspot/src/share/vm/oops/arrayOop.cpp Wed Dec 14 12:15:26 2011 +0100
@@ -38,9 +38,7 @@
return (julong)(size_t)bytes == bytes;
}
-bool arrayOopDesc::test_max_array_length() {
- tty->print_cr("test_max_array_length");
-
+void arrayOopDesc::test_max_array_length() {
assert(check_max_length_overflow(T_BOOLEAN), "size_t overflow for boolean array");
assert(check_max_length_overflow(T_CHAR), "size_t overflow for char array");
assert(check_max_length_overflow(T_FLOAT), "size_t overflow for float array");
@@ -54,8 +52,6 @@
assert(check_max_length_overflow(T_NARROWOOP), "size_t overflow for narrowOop array");
// T_VOID and T_ADDRESS are not supported by max_array_length()
-
- return true;
}
--- a/hotspot/src/share/vm/oops/arrayOop.hpp Fri Dec 09 19:28:34 2011 -0800
+++ b/hotspot/src/share/vm/oops/arrayOop.hpp Wed Dec 14 12:15:26 2011 +0100
@@ -128,7 +128,7 @@
#ifndef PRODUCT
static bool check_max_length_overflow(BasicType type);
static int32_t old_max_array_length(BasicType type);
- static bool test_max_array_length();
+ static void test_max_array_length();
#endif
};
--- a/hotspot/src/share/vm/prims/jni.cpp Fri Dec 09 19:28:34 2011 -0800
+++ b/hotspot/src/share/vm/prims/jni.cpp Wed Dec 14 12:15:26 2011 +0100
@@ -5037,16 +5037,25 @@
#ifndef PRODUCT
+#include "gc_interface/collectedHeap.hpp"
#include "utilities/quickSort.hpp"
+#define run_unit_test(unit_test_function_call) \
+ tty->print_cr("Running test: " #unit_test_function_call); \
+ unit_test_function_call
+
void execute_internal_vm_tests() {
if (ExecuteInternalVMTests) {
- assert(QuickSort::test_quick_sort(), "test_quick_sort failed");
- assert(arrayOopDesc::test_max_array_length(), "test_max_array_length failed");
+ tty->print_cr("Running internal VM tests");
+ run_unit_test(arrayOopDesc::test_max_array_length());
+ run_unit_test(CollectedHeap::test_is_in());
+ run_unit_test(QuickSort::test_quick_sort());
tty->print_cr("All internal VM tests passed");
}
}
+#undef run_unit_test
+
#endif
#ifndef USDT2
--- a/hotspot/src/share/vm/utilities/quickSort.cpp Fri Dec 09 19:28:34 2011 -0800
+++ b/hotspot/src/share/vm/utilities/quickSort.cpp Wed Dec 14 12:15:26 2011 +0100
@@ -93,8 +93,7 @@
return compare_arrays(arrayToSort, expectedResult, length);
}
-bool QuickSort::test_quick_sort() {
- tty->print_cr("test_quick_sort");
+void QuickSort::test_quick_sort() {
{
int* test_array = NULL;
int* expected_array = NULL;
@@ -214,7 +213,6 @@
delete[] test_array;
delete[] expected_array;
}
- return true;
}
#endif
--- a/hotspot/src/share/vm/utilities/quickSort.hpp Fri Dec 09 19:28:34 2011 -0800
+++ b/hotspot/src/share/vm/utilities/quickSort.hpp Wed Dec 14 12:15:26 2011 +0100
@@ -130,7 +130,7 @@
static void print_array(const char* prefix, int* array, int length);
static bool compare_arrays(int* actual, int* expected, int length);
template <class C> static bool sort_and_compare(int* arrayToSort, int* expectedResult, int length, C comparator, bool idempotent = false);
- static bool test_quick_sort();
+ static void test_quick_sort();
#endif
};