# HG changeset patch # User stefank # Date 1392033111 -3600 # Node ID 2698c3a4ebb601fd056115f6f424fb6140de79d1 # Parent 0cbd539cce7af570ca0634aba3a63d28ce05ae37 8033764: Remove the usage of StarTask from BufferingOopClosure Reviewed-by: mgerdin, brutisso, tschatzl diff -r 0cbd539cce7a -r 2698c3a4ebb6 hotspot/src/share/vm/gc_implementation/g1/bufferingOopClosure.cpp --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/hotspot/src/share/vm/gc_implementation/g1/bufferingOopClosure.cpp Mon Feb 10 12:51:51 2014 +0100 @@ -0,0 +1,271 @@ +/* + * Copyright (c) 2014, 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. + * + */ + +#include "precompiled.hpp" +#include "gc_implementation/g1/bufferingOopClosure.hpp" +#include "memory/iterator.hpp" +#include "utilities/debug.hpp" + +/////////////// Unit tests /////////////// + +#ifndef PRODUCT + +class TestBufferingOopClosure { + + // Helper class to fake a set of oop*s and narrowOop*s. + class FakeRoots { + public: + // Used for sanity checking of the values passed to the do_oops functions in the test. + static const uintptr_t NarrowOopMarker = uintptr_t(1) << (BitsPerWord -1); + + int _num_narrow; + int _num_full; + void** _narrow; + void** _full; + + FakeRoots(int num_narrow, int num_full) : + _num_narrow(num_narrow), + _num_full(num_full), + _narrow((void**)::malloc(sizeof(void*) * num_narrow)), + _full((void**)::malloc(sizeof(void*) * num_full)) { + + for (int i = 0; i < num_narrow; i++) { + _narrow[i] = (void*)(NarrowOopMarker + (uintptr_t)i); + } + for (int i = 0; i < num_full; i++) { + _full[i] = (void*)(uintptr_t)i; + } + } + + ~FakeRoots() { + ::free(_narrow); + ::free(_full); + } + + void oops_do_narrow_then_full(OopClosure* cl) { + for (int i = 0; i < _num_narrow; i++) { + cl->do_oop((narrowOop*)_narrow[i]); + } + for (int i = 0; i < _num_full; i++) { + cl->do_oop((oop*)_full[i]); + } + } + + void oops_do_full_then_narrow(OopClosure* cl) { + for (int i = 0; i < _num_full; i++) { + cl->do_oop((oop*)_full[i]); + } + for (int i = 0; i < _num_narrow; i++) { + cl->do_oop((narrowOop*)_narrow[i]); + } + } + + void oops_do_mixed(OopClosure* cl) { + int i; + for (i = 0; i < _num_full && i < _num_narrow; i++) { + cl->do_oop((oop*)_full[i]); + cl->do_oop((narrowOop*)_narrow[i]); + } + for (int j = i; j < _num_full; j++) { + cl->do_oop((oop*)_full[i]); + } + for (int j = i; j < _num_narrow; j++) { + cl->do_oop((narrowOop*)_narrow[i]); + } + } + + static const int MaxOrder = 2; + + void oops_do(OopClosure* cl, int do_oop_order) { + switch(do_oop_order) { + case 0: + oops_do_narrow_then_full(cl); + break; + case 1: + oops_do_full_then_narrow(cl); + break; + case 2: + oops_do_mixed(cl); + break; + default: + oops_do_narrow_then_full(cl); + break; + } + } + }; + + class CountOopClosure : public OopClosure { + int _narrow_oop_count; + int _full_oop_count; + public: + CountOopClosure() : _narrow_oop_count(0), _full_oop_count(0) {} + void do_oop(narrowOop* p) { + assert((uintptr_t(p) & FakeRoots::NarrowOopMarker) != 0, + "The narrowOop was unexpectedly not marked with the NarrowOopMarker"); + _narrow_oop_count++; + } + + void do_oop(oop* p){ + assert((uintptr_t(p) & FakeRoots::NarrowOopMarker) == 0, + "The oop was unexpectedly marked with the NarrowOopMarker"); + _full_oop_count++; + } + + int narrow_oop_count() { return _narrow_oop_count; } + int full_oop_count() { return _full_oop_count; } + int all_oop_count() { return _narrow_oop_count + _full_oop_count; } + }; + + class DoNothingOopClosure : public OopClosure { + public: + void do_oop(narrowOop* p) {} + void do_oop(oop* p) {} + }; + + static void testCount(int num_narrow, int num_full, int do_oop_order) { + FakeRoots fr(num_narrow, num_full); + + CountOopClosure coc; + BufferingOopClosure boc(&coc); + + fr.oops_do(&boc, do_oop_order); + + boc.done(); + + #define assert_testCount(got, expected) \ + assert((got) == (expected), \ + err_msg("Expected: %d, got: %d, when running testCount(%d, %d, %d)", \ + (got), (expected), num_narrow, num_full, do_oop_order)) + + assert_testCount(num_narrow, coc.narrow_oop_count()); + assert_testCount(num_full, coc.full_oop_count()); + assert_testCount(num_narrow + num_full, coc.all_oop_count()); + } + + static void testCount() { + int buffer_length = BufferingOopClosure::BufferLength; + + for (int order = 0; order < FakeRoots::MaxOrder; order++) { + testCount(0, 0, order); + testCount(10, 0, order); + testCount(0, 10, order); + testCount(10, 10, order); + testCount(buffer_length, 10, order); + testCount(10, buffer_length, order); + testCount(buffer_length, buffer_length, order); + testCount(buffer_length + 1, 10, order); + testCount(10, buffer_length + 1, order); + testCount(buffer_length + 1, buffer_length, order); + testCount(buffer_length, buffer_length + 1, order); + testCount(buffer_length + 1, buffer_length + 1, order); + } + } + + static void testIsBufferEmptyOrFull(int num_narrow, int num_full, bool expect_empty, bool expect_full) { + FakeRoots fr(num_narrow, num_full); + + DoNothingOopClosure cl; + BufferingOopClosure boc(&cl); + + fr.oops_do(&boc, 0); + + #define assert_testIsBufferEmptyOrFull(got, expected) \ + assert((got) == (expected), \ + err_msg("Expected: %d, got: %d. testIsBufferEmptyOrFull(%d, %d, %s, %s)", \ + (got), (expected), num_narrow, num_full, \ + BOOL_TO_STR(expect_empty), BOOL_TO_STR(expect_full))) + + assert_testIsBufferEmptyOrFull(expect_empty, boc.is_buffer_empty()); + assert_testIsBufferEmptyOrFull(expect_full, boc.is_buffer_full()); + } + + static void testIsBufferEmptyOrFull() { + int bl = BufferingOopClosure::BufferLength; + + testIsBufferEmptyOrFull(0, 0, true, false); + testIsBufferEmptyOrFull(1, 0, false, false); + testIsBufferEmptyOrFull(0, 1, false, false); + testIsBufferEmptyOrFull(1, 1, false, false); + testIsBufferEmptyOrFull(10, 0, false, false); + testIsBufferEmptyOrFull(0, 10, false, false); + testIsBufferEmptyOrFull(10, 10, false, false); + testIsBufferEmptyOrFull(0, bl, false, true); + testIsBufferEmptyOrFull(bl, 0, false, true); + testIsBufferEmptyOrFull(bl/2, bl/2, false, true); + testIsBufferEmptyOrFull(bl-1, 1, false, true); + testIsBufferEmptyOrFull(1, bl-1, false, true); + // Processed + testIsBufferEmptyOrFull(bl+1, 0, false, false); + testIsBufferEmptyOrFull(bl*2, 0, false, true); + } + + static void testEmptyAfterDone(int num_narrow, int num_full) { + FakeRoots fr(num_narrow, num_full); + + DoNothingOopClosure cl; + BufferingOopClosure boc(&cl); + + fr.oops_do(&boc, 0); + + // Make sure all get processed. + boc.done(); + + assert(boc.is_buffer_empty(), + err_msg("Should be empty after call to done(). testEmptyAfterDone(%d, %d)", + num_narrow, num_full)); + } + + static void testEmptyAfterDone() { + int bl = BufferingOopClosure::BufferLength; + + testEmptyAfterDone(0, 0); + testEmptyAfterDone(1, 0); + testEmptyAfterDone(0, 1); + testEmptyAfterDone(1, 1); + testEmptyAfterDone(10, 0); + testEmptyAfterDone(0, 10); + testEmptyAfterDone(10, 10); + testEmptyAfterDone(0, bl); + testEmptyAfterDone(bl, 0); + testEmptyAfterDone(bl/2, bl/2); + testEmptyAfterDone(bl-1, 1); + testEmptyAfterDone(1, bl-1); + // Processed + testEmptyAfterDone(bl+1, 0); + testEmptyAfterDone(bl*2, 0); + } + + public: + static void test() { + testCount(); + testIsBufferEmptyOrFull(); + testEmptyAfterDone(); + } +}; + +void TestBufferingOopClosure_test() { + TestBufferingOopClosure::test(); +} + +#endif diff -r 0cbd539cce7a -r 2698c3a4ebb6 hotspot/src/share/vm/gc_implementation/g1/bufferingOopClosure.hpp --- a/hotspot/src/share/vm/gc_implementation/g1/bufferingOopClosure.hpp Fri Feb 07 13:48:07 2014 +0100 +++ b/hotspot/src/share/vm/gc_implementation/g1/bufferingOopClosure.hpp Mon Feb 10 12:51:51 2014 +0100 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2001, 2010, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2001, 2014, 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,10 +25,10 @@ #ifndef SHARE_VM_GC_IMPLEMENTATION_G1_BUFFERINGOOPCLOSURE_HPP #define SHARE_VM_GC_IMPLEMENTATION_G1_BUFFERINGOOPCLOSURE_HPP -#include "memory/genOopClosures.hpp" -#include "memory/generation.hpp" +#include "memory/iterator.hpp" +#include "oops/oopsHierarchy.hpp" #include "runtime/os.hpp" -#include "utilities/taskqueue.hpp" +#include "utilities/debug.hpp" // A BufferingOops closure tries to separate out the cost of finding roots // from the cost of applying closures to them. It maintains an array of @@ -41,60 +41,103 @@ // The caller must be sure to call "done" to process any unprocessed // buffered entries. -class Generation; -class HeapRegion; - class BufferingOopClosure: public OopClosure { + friend class TestBufferingOopClosure; protected: - enum PrivateConstants { - BufferLength = 1024 - }; + static const size_t BufferLength = 1024; - StarTask _buffer[BufferLength]; - StarTask* _buffer_top; - StarTask* _buffer_curr; + // We need to know if the buffered addresses contain oops or narrowOops. + // We can't tag the addresses the way StarTask does, because we need to + // be able to handle unaligned addresses coming from oops embedded in code. + // + // The addresses for the full-sized oops are filled in from the bottom, + // while the addresses for the narrowOops are filled in from the top. + OopOrNarrowOopStar _buffer[BufferLength]; + OopOrNarrowOopStar* _oop_top; + OopOrNarrowOopStar* _narrowOop_bottom; OopClosure* _oc; double _closure_app_seconds; - void process_buffer () { + + bool is_buffer_empty() { + return _oop_top == _buffer && _narrowOop_bottom == (_buffer + BufferLength - 1); + } + + bool is_buffer_full() { + return _narrowOop_bottom < _oop_top; + } + + // Process addresses containing full-sized oops. + void process_oops() { + for (OopOrNarrowOopStar* curr = _buffer; curr < _oop_top; ++curr) { + _oc->do_oop((oop*)(*curr)); + } + _oop_top = _buffer; + } + + // Process addresses containing narrow oops. + void process_narrowOops() { + for (OopOrNarrowOopStar* curr = _buffer + BufferLength - 1; curr > _narrowOop_bottom; --curr) { + _oc->do_oop((narrowOop*)(*curr)); + } + _narrowOop_bottom = _buffer + BufferLength - 1; + } + + // Apply the closure to all oops and clear the buffer. + // Accumulate the time it took. + void process_buffer() { double start = os::elapsedTime(); - for (StarTask* curr = _buffer; curr < _buffer_curr; ++curr) { - if (curr->is_narrow()) { - assert(UseCompressedOops, "Error"); - _oc->do_oop((narrowOop*)(*curr)); - } else { - _oc->do_oop((oop*)(*curr)); - } - } - _buffer_curr = _buffer; + + process_oops(); + process_narrowOops(); + _closure_app_seconds += (os::elapsedTime() - start); } - template inline void do_oop_work(T* p) { - if (_buffer_curr == _buffer_top) { + void process_buffer_if_full() { + if (is_buffer_full()) { process_buffer(); } - StarTask new_ref(p); - *_buffer_curr = new_ref; - ++_buffer_curr; + } + + void add_narrowOop(narrowOop* p) { + assert(!is_buffer_full(), "Buffer should not be full"); + *_narrowOop_bottom = (OopOrNarrowOopStar)p; + _narrowOop_bottom--; + } + + void add_oop(oop* p) { + assert(!is_buffer_full(), "Buffer should not be full"); + *_oop_top = (OopOrNarrowOopStar)p; + _oop_top++; } public: - virtual void do_oop(narrowOop* p) { do_oop_work(p); } - virtual void do_oop(oop* p) { do_oop_work(p); } + virtual void do_oop(narrowOop* p) { + process_buffer_if_full(); + add_narrowOop(p); + } - void done () { - if (_buffer_curr > _buffer) { + virtual void do_oop(oop* p) { + process_buffer_if_full(); + add_oop(p); + } + + void done() { + if (!is_buffer_empty()) { process_buffer(); } } - double closure_app_seconds () { + + double closure_app_seconds() { return _closure_app_seconds; } - BufferingOopClosure (OopClosure *oc) : + + BufferingOopClosure(OopClosure *oc) : _oc(oc), - _buffer_curr(_buffer), _buffer_top(_buffer + BufferLength), + _oop_top(_buffer), + _narrowOop_bottom(_buffer + BufferLength - 1), _closure_app_seconds(0.0) { } }; diff -r 0cbd539cce7a -r 2698c3a4ebb6 hotspot/src/share/vm/gc_implementation/g1/g1RemSet.cpp --- a/hotspot/src/share/vm/gc_implementation/g1/g1RemSet.cpp Fri Feb 07 13:48:07 2014 +0100 +++ b/hotspot/src/share/vm/gc_implementation/g1/g1RemSet.cpp Mon Feb 10 12:51:51 2014 +0100 @@ -23,7 +23,6 @@ */ #include "precompiled.hpp" -#include "gc_implementation/g1/bufferingOopClosure.hpp" #include "gc_implementation/g1/concurrentG1Refine.hpp" #include "gc_implementation/g1/concurrentG1RefineThread.hpp" #include "gc_implementation/g1/g1BlockOffsetTable.inline.hpp" diff -r 0cbd539cce7a -r 2698c3a4ebb6 hotspot/src/share/vm/gc_implementation/g1/g1RemSet.hpp --- a/hotspot/src/share/vm/gc_implementation/g1/g1RemSet.hpp Fri Feb 07 13:48:07 2014 +0100 +++ b/hotspot/src/share/vm/gc_implementation/g1/g1RemSet.hpp Mon Feb 10 12:51:51 2014 +0100 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2001, 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2001, 2014, 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 diff -r 0cbd539cce7a -r 2698c3a4ebb6 hotspot/src/share/vm/prims/jni.cpp --- a/hotspot/src/share/vm/prims/jni.cpp Fri Feb 07 13:48:07 2014 +0100 +++ b/hotspot/src/share/vm/prims/jni.cpp Mon Feb 10 12:51:51 2014 +0100 @@ -4952,6 +4952,7 @@ void TestOldFreeSpaceCalculation_test(); #if INCLUDE_ALL_GCS void TestG1BiasedArray_test(); +void TestBufferingOopClosure_test(); #endif void execute_internal_vm_tests() { @@ -4977,6 +4978,7 @@ #if INCLUDE_ALL_GCS run_unit_test(TestG1BiasedArray_test()); run_unit_test(HeapRegionRemSet::test_prt()); + run_unit_test(TestBufferingOopClosure_test()); #endif tty->print_cr("All internal VM tests passed"); }