Mark more roots.
This is most of the stuff. (Everything that currently exists, though there's
more to come.)
Change-Id: I235a21b006820a027c494374a5b52ffefed89c32
diff --git a/src/class_linker.cc b/src/class_linker.cc
index da87a97..36bf6a7 100644
--- a/src/class_linker.cc
+++ b/src/class_linker.cc
@@ -426,25 +426,22 @@
// Keep in sync with InitCallback. Anything we visit, we need to
// reinit references to when reinitializing a ClassLinker from a
// mapped image.
-void ClassLinker::VisitRoots(Heap::RootVistor* root_visitor, void* arg) const {
-
- root_visitor(class_roots_, arg);
+void ClassLinker::VisitRoots(Heap::RootVisitor* visitor, void* arg) const {
+ visitor(class_roots_, arg);
for (size_t i = 0; i < dex_caches_.size(); i++) {
- root_visitor(dex_caches_[i], arg);
+ visitor(dex_caches_[i], arg);
}
{
MutexLock mu(classes_lock_);
typedef Table::const_iterator It; // TODO: C++0x auto
for (It it = classes_.begin(), end = classes_.end(); it != end; ++it) {
- root_visitor(it->second, arg);
+ visitor(it->second, arg);
}
}
- intern_table_->VisitRoots(root_visitor, arg);
-
- root_visitor(array_interfaces_, arg);
+ visitor(array_interfaces_, arg);
}
ClassLinker::~ClassLinker() {
diff --git a/src/class_linker.h b/src/class_linker.h
index 4702a57..3f95b8d 100644
--- a/src/class_linker.h
+++ b/src/class_linker.h
@@ -122,7 +122,7 @@
return boot_class_path_;
}
- void VisitRoots(Heap::RootVistor* root_visitor, void* arg) const;
+ void VisitRoots(Heap::RootVisitor* visitor, void* arg) const;
const DexFile& FindDexFile(const DexCache* dex_cache) const;
DexCache* FindDexCache(const DexFile& dex_file) const;
diff --git a/src/heap.h b/src/heap.h
index c3ea080..6420a2a 100644
--- a/src/heap.h
+++ b/src/heap.h
@@ -7,11 +7,11 @@
#include "globals.h"
#include "object_bitmap.h"
-#include "thread.h"
namespace art {
class Class;
+class Mutex;
class Object;
class Space;
class HeapBitmap;
@@ -22,7 +22,7 @@
static const size_t kMaximumSize = 64 * MB;
- typedef void (RootVistor)(const Object* root, void* arg);
+ typedef void (RootVisitor)(const Object* root, void* arg);
// Create a heap with the requested sizes. optional boot image may
// be NULL, otherwise it is an image filename created by ImageWriter.
@@ -175,19 +175,6 @@
DISALLOW_IMPLICIT_CONSTRUCTORS(Heap);
};
-class HeapLock {
- public:
- HeapLock(Heap* heap) : lock_(heap->GetLock()) {
- lock_->Lock();
- }
- ~HeapLock() {
- lock_->Unlock();
- }
- private:
- Mutex* lock_;
- DISALLOW_COPY_AND_ASSIGN(HeapLock);
-};
-
} // namespace art
#endif // ART_SRC_HEAP_H_
diff --git a/src/indirect_reference_table.cc b/src/indirect_reference_table.cc
index 32d942f..e3632a6 100644
--- a/src/indirect_reference_table.cc
+++ b/src/indirect_reference_table.cc
@@ -295,6 +295,13 @@
return true;
}
+void IndirectReferenceTable::VisitRoots(Heap::RootVisitor* visitor, void* arg) {
+ typedef IndirectReferenceTable::iterator It; // TODO: C++0x auto
+ for (It it = begin(), end = this->end(); it != end; ++it) {
+ visitor(**it, arg);
+ }
+}
+
std::ostream& operator<<(std::ostream& os, IndirectRefKind rhs) {
switch (rhs) {
case kSirtOrInvalid:
diff --git a/src/indirect_reference_table.h b/src/indirect_reference_table.h
index da5f955..a53983f 100644
--- a/src/indirect_reference_table.h
+++ b/src/indirect_reference_table.h
@@ -17,6 +17,7 @@
#ifndef ART_SRC_INDIRECT_REFERENCE_TABLE_H_
#define ART_SRC_INDIRECT_REFERENCE_TABLE_H_
+#include "heap.h"
#include "logging.h"
#include <iosfwd>
@@ -307,6 +308,8 @@
return iterator(table_, Capacity(), Capacity());
}
+ void VisitRoots(Heap::RootVisitor* visitor, void* arg);
+
private:
/*
* Extract the table index from an indirect reference.
diff --git a/src/intern_table.cc b/src/intern_table.cc
index 7d5be29..8a4e87e 100644
--- a/src/intern_table.cc
+++ b/src/intern_table.cc
@@ -20,11 +20,11 @@
return strong_interns_.size() + weak_interns_.size();
}
-void InternTable::VisitRoots(Heap::RootVistor* root_visitor, void* arg) const {
+void InternTable::VisitRoots(Heap::RootVisitor* visitor, void* arg) const {
MutexLock mu(intern_table_lock_);
typedef Table::const_iterator It; // TODO: C++0x auto
for (It it = strong_interns_.begin(), end = strong_interns_.end(); it != end; ++it) {
- root_visitor(it->second, arg);
+ visitor(it->second, arg);
}
// Note: we deliberately don't visit the weak_interns_ table.
}
@@ -116,7 +116,7 @@
return found == s;
}
-void InternTable::RemoveWeakIf(bool (*predicate)(const String*)) {
+void InternTable::RemoveWeakIf(const Predicate& predicate) {
MutexLock mu(intern_table_lock_);
typedef Table::const_iterator It; // TODO: C++0x auto
for (It it = weak_interns_.begin(), end = weak_interns_.end(); it != end;) {
diff --git a/src/intern_table.h b/src/intern_table.h
index d7e9839..c9de228 100644
--- a/src/intern_table.h
+++ b/src/intern_table.h
@@ -35,14 +35,20 @@
// Used when reinitializing InternTable from an image.
void RegisterStrong(const String* s);
- // Removes all weak interns for which 'predicate' is true.
- void RemoveWeakIf(bool (*predicate)(const String*));
+ // Removes all weak interns for which the predicate functor 'p' returns true.
+ // (We use an explicit Predicate type rather than a template to keep implementation
+ // out of the header file.)
+ struct Predicate {
+ virtual ~Predicate() {}
+ virtual bool operator()(const String*) const = 0;
+ };
+ void RemoveWeakIf(const Predicate& p);
bool ContainsWeak(const String* s);
size_t Size() const;
- void VisitRoots(Heap::RootVistor* root_visitor, void* arg) const;
+ void VisitRoots(Heap::RootVisitor* visitor, void* arg) const;
private:
typedef std::tr1::unordered_multimap<int32_t, const String*> Table;
diff --git a/src/intern_table_test.cc b/src/intern_table_test.cc
index d230a01..0362b05 100644
--- a/src/intern_table_test.cc
+++ b/src/intern_table_test.cc
@@ -36,20 +36,33 @@
EXPECT_EQ(2U, t.Size());
}
-std::vector<const String*> gExpectedWeakStrings;
-bool TestPredicate(const String* s) {
- bool erased = false;
- typedef std::vector<const String*>::iterator It; // TODO: C++0x auto
- for (It it = gExpectedWeakStrings.begin(), end = gExpectedWeakStrings.end(); it != end; ++it) {
- if (*it == s) {
- gExpectedWeakStrings.erase(it);
- erased = true;
- break;
+class TestPredicate : public InternTable::Predicate {
+ public:
+ bool operator()(const String* s) const {
+ bool erased = false;
+ typedef std::vector<const String*>::iterator It; // TODO: C++0x auto
+ for (It it = expected_.begin(), end = expected_.end(); it != end; ++it) {
+ if (*it == s) {
+ expected_.erase(it);
+ erased = true;
+ break;
+ }
}
+ EXPECT_TRUE(erased);
+ return true;
}
- EXPECT_TRUE(erased);
- return true;
-}
+
+ void Expect(const String* s) {
+ expected_.push_back(s);
+ }
+
+ ~TestPredicate() {
+ EXPECT_EQ(0U, expected_.size());
+ }
+
+ private:
+ mutable std::vector<const String*> expected_;
+};
TEST_F(InternTableTest, RemoveWeakIf) {
InternTable t;
@@ -61,11 +74,10 @@
EXPECT_EQ(4U, t.Size());
// We should traverse only the weaks...
- gExpectedWeakStrings.clear();
- gExpectedWeakStrings.push_back(s0);
- gExpectedWeakStrings.push_back(s1);
- t.RemoveWeakIf(TestPredicate);
- EXPECT_EQ(0U, gExpectedWeakStrings.size());
+ TestPredicate p;
+ p.Expect(s0);
+ p.Expect(s1);
+ t.RemoveWeakIf(p);
EXPECT_EQ(2U, t.Size());
diff --git a/src/jni_internal.cc b/src/jni_internal.cc
index 685ac2e..c9e4839 100644
--- a/src/jni_internal.cc
+++ b/src/jni_internal.cc
@@ -2862,6 +2862,18 @@
return libraries->FindNativeMethod(m);
}
+void JavaVMExt::VisitRoots(Heap::RootVisitor* visitor, void* arg) {
+ {
+ MutexLock mu(globals_lock);
+ globals.VisitRoots(visitor, arg);
+ }
+ {
+ MutexLock mu(pins_lock);
+ pin_table.VisitRoots(visitor, arg);
+ }
+ // The weak_globals table is visited by the GC itself (because it mutates the table).
+}
+
} // namespace art
std::ostream& operator<<(std::ostream& os, const jobjectRefType& rhs) {
diff --git a/src/jni_internal.h b/src/jni_internal.h
index 74c107c..11f7750 100644
--- a/src/jni_internal.h
+++ b/src/jni_internal.h
@@ -5,6 +5,7 @@
#include "jni.h"
+#include "heap.h"
#include "indirect_reference_table.h"
#include "macros.h"
#include "reference_table.h"
@@ -43,6 +44,8 @@
*/
void* FindCodeForNativeMethod(Method* m);
+ void VisitRoots(Heap::RootVisitor*, void*);
+
Runtime* runtime;
// Used for testing. By default, we'll LOG(FATAL) the reason.
diff --git a/src/mark_sweep.cc b/src/mark_sweep.cc
index 0b61b9a..af7f1a5 100644
--- a/src/mark_sweep.cc
+++ b/src/mark_sweep.cc
@@ -5,12 +5,14 @@
#include <climits>
#include <vector>
+#include "class_loader.h"
#include "heap.h"
+#include "indirect_reference_table.h"
+#include "intern_table.h"
#include "logging.h"
#include "macros.h"
#include "mark_stack.h"
#include "object.h"
-#include "class_loader.h"
#include "runtime.h"
#include "space.h"
#include "thread.h"
@@ -107,11 +109,38 @@
UNIMPLEMENTED(FATAL);
}
-void MarkSweep::SweepSystemWeaks() {
- //Runtime::Current()->GetInternTable().RemoveWeakIf(isUnmarkedObject);
+void MarkSweep::SweepJniWeakGlobals() {
+ JavaVMExt* vm = Runtime::Current()->GetJavaVM();
+ MutexLock mu(vm->weak_globals_lock);
+ IndirectReferenceTable* table = &vm->weak_globals;
+ typedef IndirectReferenceTable::iterator It; // TODO: C++0x auto
+ for (It it = table->begin(), end = table->end(); it != end; ++it) {
+ const Object** entry = *it;
+ if (!IsMarked(*entry)) {
+ *entry = kClearedJniWeakGlobal;
+ }
+ }
+}
+
+struct InternTableEntryIsUnmarked : public InternTable::Predicate {
+ InternTableEntryIsUnmarked(MarkSweep* ms) : ms_(ms) { }
+
+ bool operator()(const String* s) const {
+ return !ms_->IsMarked(s);
+ }
+
+ MarkSweep* ms_;
+};
+
+void MarkSweep::SweepMonitorList() {
UNIMPLEMENTED(FATAL);
//dvmSweepMonitorList(&gDvm.monitorList, isUnmarkedObject);
- //sweepWeakJniGlobals();
+}
+
+void MarkSweep::SweepSystemWeaks() {
+ Runtime::Current()->GetInternTable()->RemoveWeakIf(InternTableEntryIsUnmarked(this));
+ SweepMonitorList();
+ SweepJniWeakGlobals();
}
void MarkSweep::SweepCallback(size_t num_ptrs, void **ptrs, void *arg) {
diff --git a/src/mark_sweep.h b/src/mark_sweep.h
index 047e357..f20cc41 100644
--- a/src/mark_sweep.h
+++ b/src/mark_sweep.h
@@ -127,6 +127,8 @@
Object** phantom_references);
void SweepSystemWeaks();
+ void SweepMonitorList();
+ void SweepJniWeakGlobals();
MarkStack* mark_stack_;
@@ -148,6 +150,8 @@
Object* cleared_reference_list_;
+ friend class InternTableEntryIsUnmarked;
+
DISALLOW_COPY_AND_ASSIGN(MarkSweep);
};
diff --git a/src/reference_table.cc b/src/reference_table.cc
index a006b65..df3daf2 100644
--- a/src/reference_table.cc
+++ b/src/reference_table.cc
@@ -135,7 +135,7 @@
Dump(entries_);
}
-void ReferenceTable::Dump(const std::vector<const Object*>& entries) {
+void ReferenceTable::Dump(const Table& entries) {
if (entries.empty()) {
LOG(WARNING) << " (empty)";
return;
@@ -195,7 +195,7 @@
}
// Make a copy of the table and sort it.
- std::vector<const Object*> sorted_entries(entries.begin(), entries.end());
+ Table sorted_entries(entries.begin(), entries.end());
std::sort(sorted_entries.begin(), sorted_entries.end(), ObjectComparator());
// Remove any uninteresting stuff from the list. The sort moved them all to the end.
@@ -233,4 +233,11 @@
LogSummaryLine(sorted_entries.back(), GetElementCount(sorted_entries.back()), identical, equiv);
}
+void ReferenceTable::VisitRoots(Heap::RootVisitor* visitor, void* arg) {
+ typedef Table::const_iterator It; // TODO: C++0x auto
+ for (It it = entries_.begin(), end = entries_.end(); it != end; ++it) {
+ visitor(*it, arg);
+ }
+}
+
} // namespace art
diff --git a/src/reference_table.h b/src/reference_table.h
index 13317ea..9142b7c 100644
--- a/src/reference_table.h
+++ b/src/reference_table.h
@@ -22,6 +22,8 @@
#include <string>
#include <vector>
+#include "heap.h"
+
namespace art {
class Object;
@@ -43,12 +45,15 @@
void Dump() const;
+ void VisitRoots(Heap::RootVisitor* visitor, void* arg);
+
private:
- static void Dump(const std::vector<const Object*>& entries);
+ typedef std::vector<const Object*> Table;
+ static void Dump(const Table& entries);
friend class IndirectReferenceTable; // For Dump.
std::string name_;
- std::vector<const Object*> entries_;
+ Table entries_;
size_t max_size_;
};
diff --git a/src/runtime.cc b/src/runtime.cc
index 985cdf6..0e5da23 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -433,9 +433,16 @@
return true;
}
-void Runtime::VisitRoots(Heap::RootVistor* root_visitor, void* arg) const {
- GetClassLinker()->VisitRoots(root_visitor, arg);
- UNIMPLEMENTED(WARNING) << "mark other roots including per thread state and thread stacks";
+void Runtime::VisitRoots(Heap::RootVisitor* visitor, void* arg) const {
+ class_linker_->VisitRoots(visitor, arg);
+ intern_table_->VisitRoots(visitor, arg);
+ java_vm_->VisitRoots(visitor, arg);
+ thread_list_->VisitRoots(visitor, arg);
+
+ //(*visitor)(&gDvm.outOfMemoryObj, 0, ROOT_VM_INTERNAL, arg);
+ //(*visitor)(&gDvm.internalErrorObj, 0, ROOT_VM_INTERNAL, arg);
+ //(*visitor)(&gDvm.noClassDefFoundErrorObj, 0, ROOT_VM_INTERNAL, arg);
+ UNIMPLEMENTED(WARNING) << "some roots not marked";
}
} // namespace art
diff --git a/src/runtime.h b/src/runtime.h
index 922eeab..6a5d755 100644
--- a/src/runtime.h
+++ b/src/runtime.h
@@ -101,7 +101,7 @@
return java_vm_;
}
- void VisitRoots(Heap::RootVistor* root_visitor, void* arg) const;
+ void VisitRoots(Heap::RootVisitor* visitor, void* arg) const;
private:
static void PlatformAbort(const char*, int);
diff --git a/src/thread.cc b/src/thread.cc
index 89ec844..5a612dd 100644
--- a/src/thread.cc
+++ b/src/thread.cc
@@ -522,6 +522,15 @@
}
}
+void Thread::VisitRoots(Heap::RootVisitor* visitor, void* arg) const {
+ //(*visitor)(&thread->threadObj, threadId, ROOT_THREAD_OBJECT, arg);
+ //(*visitor)(&thread->exception, threadId, ROOT_NATIVE_STACK, arg);
+ jni_env_->locals.VisitRoots(visitor, arg);
+ jni_env_->monitors.VisitRoots(visitor, arg);
+ // visitThreadStack(visitor, thread, arg);
+ UNIMPLEMENTED(WARNING) << "some per-Thread roots not visited";
+}
+
static const char* kStateNames[] = {
"New",
"Runnable",
@@ -588,4 +597,12 @@
list_.remove(thread);
}
+void ThreadList::VisitRoots(Heap::RootVisitor* visitor, void* arg) const {
+ MutexLock mu(lock_);
+ typedef std::list<Thread*>::const_iterator It; // TODO: C++0x auto
+ for (It it = list_.begin(), end = list_.end(); it != end; ++it) {
+ (*it)->VisitRoots(visitor, arg);
+ }
+}
+
} // namespace
diff --git a/src/thread.h b/src/thread.h
index ab0660e..ae07fa6 100644
--- a/src/thread.h
+++ b/src/thread.h
@@ -406,6 +406,8 @@
// Allocate stack trace
ObjectArray<StackTraceElement>* AllocStackTrace();
+ void VisitRoots(Heap::RootVisitor* visitor, void* arg) const;
+
private:
Thread()
: id_(1234),
@@ -509,6 +511,8 @@
lock_->Unlock();
};
+ void VisitRoots(Heap::RootVisitor* visitor, void* arg) const;
+
private:
ThreadList();