Separate HPROF from the GC.

In the beginning, the only way to traverse the roots and heap was to
piggyback off the garbage collector.  As such, HPROF was implemented
by instrumenting the root- and object traversal routines to check a
mode flag and call into HPROF during a GC when the flag was set.

This change moves the HPROF calls out of the GC and into callbacks
invoked through the visitor.  Notably, it allows HPROF dumps to be
computed at any point in time without invoking a GC and potentially
destroying evidence relating to the cause of an OOM.

Change-Id: I2b74c4f10f35af3ca33b7c0bbfe470a8b586ff66

13 files changed, 194 insertions, 219 deletions

diff --git a/vm/Thread.c b/vm/Thread.c
index a98dbd000..54d0f4bdd 100644
--- a/vm/Thread.c
+++ b/vm/Thread.c
@@ -534,6 +534,7 @@ static const char* getSuspendCauseStr(SuspendCause why)
     case SUSPEND_FOR_DEBUG_EVENT:   return "debug-event";
     case SUSPEND_FOR_STACK_DUMP:    return "stack-dump";
     case SUSPEND_FOR_VERIFY:        return "verify";
+    case SUSPEND_FOR_HPROF:         return "hprof";
 #if defined(WITH_JIT)
     case SUSPEND_FOR_TBL_RESIZE:    return "table-resize";
     case SUSPEND_FOR_IC_PATCH:      return "inline-cache-patch";
@@ -4180,17 +4181,11 @@ static void gcScanThread(Thread *thread)
         /* continue anyway */
     }
 
-    HPROF_SET_GC_SCAN_STATE(HPROF_ROOT_THREAD_OBJECT, thread->threadId);
-
     dvmMarkObject(thread->threadObj);   // could be NULL, when constructing
 
-    HPROF_SET_GC_SCAN_STATE(HPROF_ROOT_NATIVE_STACK, thread->threadId);
-
     dvmMarkObject(thread->exception);   // usually NULL
     gcScanReferenceTable(&thread->internalLocalRefTable);
 
-    HPROF_SET_GC_SCAN_STATE(HPROF_ROOT_JNI_LOCAL, thread->threadId);
-
 #ifdef USE_INDIRECT_REF
     gcScanIndirectRefTable(&thread->jniLocalRefTable);
 #else
@@ -4198,16 +4193,10 @@ static void gcScanThread(Thread *thread)
 #endif
 
     if (thread->jniMonitorRefTable.table != NULL) {
-        HPROF_SET_GC_SCAN_STATE(HPROF_ROOT_JNI_MONITOR, thread->threadId);
-
         gcScanReferenceTable(&thread->jniMonitorRefTable);
     }
 
-    HPROF_SET_GC_SCAN_STATE(HPROF_ROOT_JAVA_FRAME, thread->threadId);
-
     gcScanInterpStackReferences(thread);
-
-    HPROF_CLEAR_GC_SCAN_STATE();
 }
 
 static void gcScanAllThreads()
diff --git a/vm/Thread.h b/vm/Thread.h
index 5afeefe37..ce53c3391 100644
--- a/vm/Thread.h
+++ b/vm/Thread.h
@@ -289,6 +289,7 @@ typedef enum SuspendCause {
     SUSPEND_FOR_STACK_DUMP,
     SUSPEND_FOR_DEX_OPT,
     SUSPEND_FOR_VERIFY,
+    SUSPEND_FOR_HPROF,
 #if defined(WITH_JIT)
     SUSPEND_FOR_TBL_RESIZE,  // jit-table resize
     SUSPEND_FOR_IC_PATCH,    // polymorphic callsite inline-cache patch
diff --git a/vm/alloc/CardTable.c b/vm/alloc/CardTable.c
index 240e6575b..4f90371a6 100644
--- a/vm/alloc/CardTable.c
+++ b/vm/alloc/CardTable.c
@@ -228,7 +228,8 @@ static void dumpReferencesCallback(void *ptr, void *arg)
 /*
  * Root visitor that looks for matching references.
  */
-static void dumpReferencesRootVisitor(void *ptr, void *arg)
+static void dumpReferencesRootVisitor(void *ptr, u4 threadId,
+                                      RootType type, void *arg)
 {
     Object *obj = *(Object **)ptr;
     Object *lookingFor = *(Object **)arg;
diff --git a/vm/alloc/GC.h b/vm/alloc/GC.h
index 62e9aa616..c65c807a4 100644
--- a/vm/alloc/GC.h
+++ b/vm/alloc/GC.h
@@ -138,18 +138,4 @@ void dvmGcMarkJniGlobalRefs(void);
  */
 void dvmGcMarkDebuggerRefs(void);
 
-/*
- * Optional heap profiling.
- */
-#if WITH_HPROF && !defined(_DALVIK_HPROF_HPROF)
-#include "hprof/Hprof.h"
-#define HPROF_SET_GC_SCAN_STATE(tag_, thread_) \
-    dvmHeapSetHprofGcScanState((tag_), (thread_))
-#define HPROF_CLEAR_GC_SCAN_STATE() \
-    dvmHeapSetHprofGcScanState(0, 0)
-#else
-#define HPROF_SET_GC_SCAN_STATE(tag_, thread_)  do {} while (false)
-#define HPROF_CLEAR_GC_SCAN_STATE()  do {} while (false)
-#endif
-
 #endif  // _DALVIK_ALLOC_GC
diff --git a/vm/alloc/Heap.c b/vm/alloc/Heap.c
index 843ee385a..bd782bd5d 100644
--- a/vm/alloc/Heap.c
+++ b/vm/alloc/Heap.c
@@ -25,7 +25,6 @@
 #include "alloc/DdmHeap.h"
 #include "alloc/HeapSource.h"
 #include "alloc/MarkSweep.h"
-#include "alloc/Visit.h"
 
 #include "utils/threads.h"      // need Android thread priorities
 #define kInvalidPriority        10000
@@ -41,8 +40,7 @@ static const char* GcReasonStr[] = {
     [GC_FOR_MALLOC] = "GC_FOR_MALLOC",
     [GC_CONCURRENT] = "GC_CONCURRENT",
     [GC_EXPLICIT] = "GC_EXPLICIT",
-    [GC_EXTERNAL_ALLOC] = "GC_EXTERNAL_ALLOC",
-    [GC_HPROF_DUMP_HEAP] = "GC_HPROF_DUMP_HEAP"
+    [GC_EXTERNAL_ALLOC] = "GC_EXTERNAL_ALLOC"
 };
 
 /*
@@ -71,10 +69,6 @@ bool dvmHeapStartup()
     gcHeap->ddmHpsgWhat = 0;
     gcHeap->ddmNhsgWhen = 0;
     gcHeap->ddmNhsgWhat = 0;
-#if WITH_HPROF
-    gcHeap->hprofDumpOnGc = false;
-    gcHeap->hprofContext = NULL;
-#endif
     gDvm.gcHeap = gcHeap;
 
     /* Set up the lists and lock we'll use for finalizable
@@ -661,46 +655,6 @@ void dvmCollectGarbageInternal(bool clearSoftRefs, GcReason reason)
 
     dvmMethodTraceGCBegin();
 
-#if WITH_HPROF
-
-/* Set DUMP_HEAP_ON_DDMS_UPDATE to 1 to enable heap dumps
- * whenever DDMS requests a heap update (HPIF chunk).
- * The output files will appear in /data/misc, which must
- * already exist.
- * You must define "WITH_HPROF := true" in your buildspec.mk
- * and recompile libdvm for this to work.
- *
- * To enable stack traces for each allocation, define
- * "WITH_HPROF_STACK := true" in buildspec.mk.  This option slows down
- * allocations and also requires 8 additional bytes per object on the
- * GC heap.
- */
-#define DUMP_HEAP_ON_DDMS_UPDATE 0
-#if DUMP_HEAP_ON_DDMS_UPDATE
-    gcHeap->hprofDumpOnGc |= (gcHeap->ddmHpifWhen != 0);
-#endif
-
-    if (gcHeap->hprofDumpOnGc) {
-        char nameBuf[128];
-
-        gcHeap->hprofResult = -1;
-
-        if (gcHeap->hprofFileName == NULL) {
-            /* no filename was provided; invent one */
-            sprintf(nameBuf, "/data/misc/heap-dump-tm%d-pid%d.hprof",
-                (int) time(NULL), (int) getpid());
-            gcHeap->hprofFileName = nameBuf;
-        }
-        gcHeap->hprofContext = hprofStartup(gcHeap->hprofFileName,
-                gcHeap->hprofFd, gcHeap->hprofDirectToDdms);
-        if (gcHeap->hprofContext != NULL) {
-            hprofStartHeapDump(gcHeap->hprofContext);
-        }
-        gcHeap->hprofDumpOnGc = false;
-        gcHeap->hprofFileName = NULL;
-    }
-#endif
-
     /* Set up the marking context.
      */
     if (!dvmHeapBeginMarkStep(gcMode)) {
@@ -852,16 +806,6 @@ void dvmCollectGarbageInternal(bool clearSoftRefs, GcReason reason)
     currAllocated = dvmHeapSourceGetValue(HS_BYTES_ALLOCATED, NULL, 0);
     currFootprint = dvmHeapSourceGetValue(HS_FOOTPRINT, NULL, 0);
 
-#if WITH_HPROF
-    if (gcHeap->hprofContext != NULL) {
-        hprofFinishHeapDump(gcHeap->hprofContext);
-//TODO: write a HEAP_SUMMARY record
-        if (hprofShutdown(gcHeap->hprofContext))
-            gcHeap->hprofResult = 0;    /* indicate success */
-        gcHeap->hprofContext = NULL;
-    }
-#endif
-
     /* Now that we've freed up the GC heap, return any large
      * free chunks back to the system.  They'll get paged back
      * in the next time they're used.  Don't do it immediately,
@@ -963,45 +907,3 @@ void dvmWaitForConcurrentGcToComplete(void)
     dvmWaitCond(&gDvm.gcHeapCond, &gDvm.gcHeapLock);
     dvmChangeStatus(self, oldStatus);
 }
-
-#if WITH_HPROF
-/*
- * Perform garbage collection, writing heap information to the specified file.
- *
- * If "fd" is >= 0, the output will be written to that file descriptor.
- * Otherwise, "fileName" is used to create an output file.
- *
- * If "fileName" is NULL, a suitable name will be generated automatically.
- * (TODO: remove this when the SIGUSR1 feature goes away)
- *
- * If "directToDdms" is set, the other arguments are ignored, and data is
- * sent directly to DDMS.
- *
- * Returns 0 on success, or an error code on failure.
- */
-int hprofDumpHeap(const char* fileName, int fd, bool directToDdms)
-{
-    int result;
-
-    dvmLockMutex(&gDvm.gcHeapLock);
-
-    gDvm.gcHeap->hprofDumpOnGc = true;
-    gDvm.gcHeap->hprofFileName = fileName;
-    gDvm.gcHeap->hprofFd = fd;
-    gDvm.gcHeap->hprofDirectToDdms = directToDdms;
-    dvmCollectGarbageInternal(false, GC_HPROF_DUMP_HEAP);
-    result = gDvm.gcHeap->hprofResult;
-
-    dvmUnlockMutex(&gDvm.gcHeapLock);
-
-    return result;
-}
-
-void dvmHeapSetHprofGcScanState(hprof_heap_tag_t state, u4 threadSerialNumber)
-{
-    if (gDvm.gcHeap->hprofContext != NULL) {
-        hprofSetGcScanState(gDvm.gcHeap->hprofContext, state,
-                threadSerialNumber);
-    }
-}
-#endif
diff --git a/vm/alloc/HeapInternal.h b/vm/alloc/HeapInternal.h
index 0298f842e..5a7e43d29 100644
--- a/vm/alloc/HeapInternal.h
+++ b/vm/alloc/HeapInternal.h
@@ -110,15 +110,6 @@ struct GcHeap {
     int             ddmHpsgWhat;
     int             ddmNhsgWhen;
     int             ddmNhsgWhat;
-
-#if WITH_HPROF
-    bool            hprofDumpOnGc;
-    const char*     hprofFileName;
-    int             hprofFd;
-    hprof_context_t *hprofContext;
-    int             hprofResult;
-    bool            hprofDirectToDdms;
-#endif
 };
 
 bool dvmLockHeap(void);
diff --git a/vm/alloc/MarkSweep.c b/vm/alloc/MarkSweep.c
index 91365adbe..84c111a13 100644
--- a/vm/alloc/MarkSweep.c
+++ b/vm/alloc/MarkSweep.c
@@ -131,12 +131,6 @@ static void markObjectNonNull(const Object *obj, GcMarkContext *ctx,
              */
             MARK_STACK_PUSH(ctx->stack, obj);
         }
-
-#if WITH_HPROF
-        if (gDvm.gcHeap->hprofContext != NULL) {
-            hprofMarkRootObject(gDvm.gcHeap->hprofContext, obj, 0);
-        }
-#endif
     }
 }
 
@@ -198,8 +192,6 @@ void dvmHeapMarkRootSet()
 {
     GcHeap *gcHeap = gDvm.gcHeap;
 
-    HPROF_SET_GC_SCAN_STATE(HPROF_ROOT_STICKY_CLASS, 0);
-
     LOG_SCAN("immune objects");
     dvmMarkImmuneObjects(gcHeap->markContext.immuneLimit);
 
@@ -208,41 +200,24 @@ void dvmHeapMarkRootSet()
     LOG_SCAN("primitive classes\n");
     dvmGcScanPrimitiveClasses();
 
-    /* dvmGcScanRootThreadGroups() sets a bunch of
-     * different scan states internally.
-     */
-    HPROF_CLEAR_GC_SCAN_STATE();
-
     LOG_SCAN("root thread groups\n");
     dvmGcScanRootThreadGroups();
 
-    HPROF_SET_GC_SCAN_STATE(HPROF_ROOT_INTERNED_STRING, 0);
-
     LOG_SCAN("interned strings\n");
     dvmGcScanInternedStrings();
 
-    HPROF_SET_GC_SCAN_STATE(HPROF_ROOT_JNI_GLOBAL, 0);
-
     LOG_SCAN("JNI global refs\n");
     dvmGcMarkJniGlobalRefs();
 
-    HPROF_SET_GC_SCAN_STATE(HPROF_ROOT_REFERENCE_CLEANUP, 0);
-
     LOG_SCAN("pending reference operations\n");
     dvmHeapMarkLargeTableRefs(gcHeap->referenceOperations);
 
-    HPROF_SET_GC_SCAN_STATE(HPROF_ROOT_FINALIZING, 0);
-
     LOG_SCAN("pending finalizations\n");
     dvmHeapMarkLargeTableRefs(gcHeap->pendingFinalizationRefs);
 
-    HPROF_SET_GC_SCAN_STATE(HPROF_ROOT_DEBUGGER, 0);
-
     LOG_SCAN("debugger refs\n");
     dvmGcMarkDebuggerRefs();
 
-    HPROF_SET_GC_SCAN_STATE(HPROF_ROOT_VM_INTERNAL, 0);
-
     /* Mark any special objects we have sitting around.
      */
     LOG_SCAN("special objects\n");
@@ -250,15 +225,13 @@ void dvmHeapMarkRootSet()
     dvmMarkObjectNonNull(gDvm.internalErrorObj);
     dvmMarkObjectNonNull(gDvm.noClassDefFoundErrorObj);
 //TODO: scan object references sitting in gDvm;  use pointer begin & end
-
-    HPROF_CLEAR_GC_SCAN_STATE();
 }
 
 /*
  * Callback applied to root references.  If the root location contains
  * a white reference it is pushed on the mark stack and grayed.
  */
-static void markObjectVisitor(void *addr, void *arg)
+static void markObjectVisitor(void *addr, RootType type, u4 thread, void *arg)
 {
     Object *obj;
 
@@ -519,11 +492,6 @@ static void scanObject(const Object *obj, GcMarkContext *ctx)
     assert(obj != NULL);
     assert(ctx != NULL);
     assert(obj->clazz != NULL);
-#if WITH_HPROF
-    if (gDvm.gcHeap->hprofContext != NULL) {
-        hprofDumpHeapObject(gDvm.gcHeap->hprofContext, obj);
-    }
-#endif
     /* Dispatch a type-specific scan routine. */
     if (obj->clazz == gDvm.classJavaLangClass) {
         scanClassObject((ClassObject *)obj, ctx);
@@ -986,13 +954,11 @@ void dvmHeapScheduleFinalizations()
     ref = newPendingRefs.table;
     lastRef = newPendingRefs.nextEntry;
     assert(ref < lastRef);
-    HPROF_SET_GC_SCAN_STATE(HPROF_ROOT_FINALIZING, 0);
     while (ref < lastRef) {
         assert(*ref != NULL);
         markObject(*ref, ctx);
         ref++;
     }
-    HPROF_CLEAR_GC_SCAN_STATE();
     processMarkStack(ctx);
     dvmSignalHeapWorker(false);
 }
diff --git a/vm/alloc/Verify.c b/vm/alloc/Verify.c
index a48ff2982..5ce692cb1 100644
--- a/vm/alloc/Verify.c
+++ b/vm/alloc/Verify.c
@@ -42,7 +42,8 @@ static void dumpReferencesCallback(void *ptr, void *arg)
     }
 }
 
-static void dumpReferencesRootVisitor(void *ptr, void *arg)
+static void dumpReferencesRootVisitor(void *ptr, u4 threadId,
+                                      RootType type, void *arg)
 {
     Object *obj = *(Object **)ptr;
     Object *lookingFor = *(Object **)arg;
@@ -120,9 +121,18 @@ void dvmVerifyBitmap(const HeapBitmap *bitmap)
 }
 
 /*
+ * Helper function to call verifyReference from the root verifier.
+ */
+static void verifyRootReference(void *addr, u4 threadId,
+                                RootType type, void *arg)
+{
+    verifyReference(addr, arg);
+}
+
+/*
  * Verifies references in the roots.
  */
 void dvmVerifyRoots(void)
 {
-    dvmVisitRoots(verifyReference, NULL);
+    dvmVisitRoots(verifyRootReference, NULL);
 }
diff --git a/vm/alloc/Visit.c b/vm/alloc/Visit.c
index 0af27dacf..537117758 100644
--- a/vm/alloc/Visit.c
+++ b/vm/alloc/Visit.c
@@ -34,7 +34,8 @@ void dvmVisitObject(Visitor *visitor, Object *obj, void *arg)
 /*
  * Applies a verification function to all present values in the hash table.
  */
-static void visitHashTable(Visitor *visitor, HashTable *table, void *arg)
+static void visitHashTable(RootVisitor *visitor, HashTable *table,
+                           RootType type, void *arg)
 {
     int i;
 
@@ -44,17 +45,32 @@ static void visitHashTable(Visitor *visitor, HashTable *table, void *arg)
     for (i = 0; i < table->tableSize; ++i) {
         HashEntry *entry = &table->pEntries[i];
         if (entry->data != NULL && entry->data != HASH_TOMBSTONE) {
-            (*visitor)(&entry->data, arg);
+            (*visitor)(&entry->data, 0, type, arg);
         }
     }
     dvmHashTableUnlock(table);
 }
 
 /*
+ * Applies a verification function to all elements in the array.
+ */
+static void visitArray(RootVisitor *visitor, Object **array, size_t length,
+                       RootType type, void *arg)
+{
+    size_t i;
+
+    assert(visitor != NULL);
+    assert(array != NULL);
+    for (i = 0; i < length; ++i) {
+        (*visitor)(&array[i], 0, type, arg);
+    }
+}
+
+/*
  * Visits all entries in the reference table.
  */
-static void visitReferenceTable(Visitor *visitor, const ReferenceTable *table,
-                                void *arg)
+static void visitReferenceTable(RootVisitor *visitor, ReferenceTable *table,
+                                u4 threadId, RootType type, void *arg)
 {
     Object **entry;
 
@@ -62,7 +78,7 @@ static void visitReferenceTable(Visitor *visitor, const ReferenceTable *table,
     assert(table != NULL);
     for (entry = table->table; entry < table->nextEntry; ++entry) {
         assert(entry != NULL);
-        (*visitor)(entry, arg);
+        (*visitor)(entry, threadId, type, arg);
     }
 }
 
@@ -70,29 +86,32 @@ static void visitReferenceTable(Visitor *visitor, const ReferenceTable *table,
  * Visits a large heap reference table.  These objects are list heads.
  * As such, it is valid for table to be NULL.
  */
-static void visitLargeHeapRefTable(Visitor *visitor, LargeHeapRefTable *table,
-                                   void *arg)
+static void visitLargeHeapRefTable(RootVisitor *visitor,
+                                   LargeHeapRefTable *table,
+                                   RootType type, void *arg)
 {
     assert(visitor != NULL);
     for (; table != NULL; table = table->next) {
-        visitReferenceTable(visitor, &table->refs, arg);
+        visitReferenceTable(visitor, &table->refs, 0, type, arg);
     }
 }
 
 /*
  * Visits all stack slots. TODO: visit native methods.
  */
-static void visitThreadStack(Visitor *visitor, Thread *thread, void *arg)
+static void visitThreadStack(RootVisitor *visitor, Thread *thread, void *arg)
 {
     const StackSaveArea *saveArea;
-    u4 *framePtr;
+    u4 *fp;
+    u4 threadId;
 
     assert(visitor != NULL);
     assert(thread != NULL);
-    framePtr = (u4 *)thread->curFrame;
-    for (; framePtr != NULL; framePtr = saveArea->prevFrame) {
+    threadId = thread->threadId;
+    fp = (u4 *)thread->curFrame;
+    for (; fp != NULL; fp = saveArea->prevFrame) {
         Method *method;
-        saveArea = SAVEAREA_FROM_FP(framePtr);
+        saveArea = SAVEAREA_FROM_FP(fp);
         method = (Method *)saveArea->method;
         if (method != NULL && !dvmIsNativeMethod(method)) {
             const RegisterMap* pMap = dvmGetExpandedRegisterMap(method);
@@ -111,8 +130,8 @@ static void visitThreadStack(Visitor *visitor, Thread *thread, void *arg)
                  * scan.
                  */
                 for (i = 0; i < method->registersSize; ++i) {
-                    if (dvmIsValidObject((Object *)framePtr[i])) {
-                        (*visitor)(&framePtr[i], arg);
+                    if (dvmIsValidObject((Object *)fp[i])) {
+                        (*visitor)(&fp[i], threadId, ROOT_JAVA_FRAME, arg);
                     }
                 }
             } else {
@@ -135,7 +154,7 @@ static void visitThreadStack(Visitor *visitor, Thread *thread, void *arg)
                         /*
                          * Register is marked as live, it's a valid root.
                          */
-                        (*visitor)(&framePtr[i], arg);
+                        (*visitor)(&fp[i], threadId, ROOT_JAVA_FRAME, arg);
                     }
                 }
                 dvmReleaseRegisterMapLine(pMap, regVector);
@@ -144,7 +163,7 @@ static void visitThreadStack(Visitor *visitor, Thread *thread, void *arg)
         /*
          * Don't fall into an infinite loop if things get corrupted.
          */
-        assert((uintptr_t)saveArea->prevFrame > (uintptr_t)framePtr ||
+        assert((uintptr_t)saveArea->prevFrame > (uintptr_t)fp ||
                saveArea->prevFrame == NULL);
     }
 }
@@ -152,16 +171,19 @@ static void visitThreadStack(Visitor *visitor, Thread *thread, void *arg)
 /*
  * Visits all roots associated with a thread.
  */
-static void visitThread(Visitor *visitor, Thread *thread, void *arg)
+static void visitThread(RootVisitor *visitor, Thread *thread, void *arg)
 {
+    u4 threadId;
+
     assert(visitor != NULL);
     assert(thread != NULL);
-    (*visitor)(&thread->threadObj, arg);
-    (*visitor)(&thread->exception, arg);
-    visitReferenceTable(visitor, &thread->internalLocalRefTable, arg);
-    visitReferenceTable(visitor, &thread->jniLocalRefTable, arg);
-    if (thread->jniMonitorRefTable.table) {
-        visitReferenceTable(visitor, &thread->jniMonitorRefTable, arg);
+    threadId = thread->threadId;
+    (*visitor)(&thread->threadObj, threadId, ROOT_THREAD_OBJECT, arg);
+    (*visitor)(&thread->exception, threadId, ROOT_NATIVE_STACK, arg);
+    visitReferenceTable(visitor, &thread->internalLocalRefTable, threadId, ROOT_NATIVE_STACK, arg);
+    visitReferenceTable(visitor, &thread->jniLocalRefTable, threadId, ROOT_JNI_LOCAL, arg);
+    if (thread->jniMonitorRefTable.table != NULL) {
+        visitReferenceTable(visitor, &thread->jniMonitorRefTable, threadId, ROOT_JNI_MONITOR, arg);
     }
     visitThreadStack(visitor, thread, arg);
 }
@@ -169,7 +191,7 @@ static void visitThread(Visitor *visitor, Thread *thread, void *arg)
 /*
  * Visits all threads on the thread list.
  */
-static void visitThreads(Visitor *visitor, void *arg)
+static void visitThreads(RootVisitor *visitor, void *arg)
 {
     Thread *thread;
 
@@ -184,22 +206,22 @@ static void visitThreads(Visitor *visitor, void *arg)
 }
 
 /*
- * Visits roots.  TODO: visit all roots.
+ * Visits roots.  TODO: visit cached global references.
  */
-void dvmVisitRoots(Visitor *visitor, void *arg)
+void dvmVisitRoots(RootVisitor *visitor, void *arg)
 {
     assert(visitor != NULL);
-    visitHashTable(visitor, gDvm.loadedClasses, arg);
-    visitHashTable(visitor, gDvm.dbgRegistry, arg);
-    visitHashTable(visitor, gDvm.internedStrings, arg);
-    visitHashTable(visitor, gDvm.literalStrings, arg);
-    visitReferenceTable(visitor, &gDvm.jniGlobalRefTable, arg);
-    visitReferenceTable(visitor, &gDvm.jniPinRefTable, arg);
-    visitLargeHeapRefTable(visitor, gDvm.gcHeap->referenceOperations, arg);
-    visitLargeHeapRefTable(visitor, gDvm.gcHeap->pendingFinalizationRefs, arg);
+    visitHashTable(visitor, gDvm.loadedClasses, ROOT_STICKY_CLASS, arg);
+    visitArray(visitor, (Object **)gDvm.primitiveClass, NELEM(gDvm.primitiveClass), ROOT_STICKY_CLASS, arg);
+    visitHashTable(visitor, gDvm.dbgRegistry, ROOT_DEBUGGER, arg);
+    visitHashTable(visitor, gDvm.internedStrings, ROOT_INTERNED_STRING, arg);
+    visitHashTable(visitor, gDvm.literalStrings, ROOT_INTERNED_STRING, arg);
+    visitReferenceTable(visitor, &gDvm.jniGlobalRefTable, 0, ROOT_JNI_GLOBAL, arg);
+    visitReferenceTable(visitor, &gDvm.jniPinRefTable, 0, ROOT_NATIVE_STACK, arg);
+    visitLargeHeapRefTable(visitor, gDvm.gcHeap->referenceOperations, ROOT_REFERENCE_CLEANUP, arg);
+    visitLargeHeapRefTable(visitor, gDvm.gcHeap->pendingFinalizationRefs, ROOT_FINALIZING, arg);
     visitThreads(visitor, arg);
-    (*visitor)(&gDvm.outOfMemoryObj, arg);
-    (*visitor)(&gDvm.internalErrorObj, arg);
-    (*visitor)(&gDvm.noClassDefFoundErrorObj, arg);
-    /* TODO: visit cached global references. */
+    (*visitor)(&gDvm.outOfMemoryObj, 0, ROOT_VM_INTERNAL, arg);
+    (*visitor)(&gDvm.internalErrorObj, 0, ROOT_VM_INTERNAL, arg);
+    (*visitor)(&gDvm.noClassDefFoundErrorObj, 0, ROOT_VM_INTERNAL, arg);
 }
diff --git a/vm/alloc/Visit.h b/vm/alloc/Visit.h
index 488c7212e..e7c52e58f 100644
--- a/vm/alloc/Visit.h
+++ b/vm/alloc/Visit.h
@@ -19,6 +19,24 @@
 
 #include "Dalvik.h"
 
+typedef enum {
+  ROOT_UNKNOWN = 0,
+  ROOT_JNI_GLOBAL,
+  ROOT_JNI_LOCAL,
+  ROOT_JAVA_FRAME,
+  ROOT_NATIVE_STACK,
+  ROOT_STICKY_CLASS,
+  ROOT_THREAD_BLOCK,
+  ROOT_MONITOR_USED,
+  ROOT_THREAD_OBJECT,
+  ROOT_INTERNED_STRING,
+  ROOT_FINALIZING,
+  ROOT_DEBUGGER,
+  ROOT_REFERENCE_CLEANUP,
+  ROOT_VM_INTERNAL,
+  ROOT_JNI_MONITOR,
+} RootType;
+
 /*
  * Callback invoked with the address of a reference and a user
  * supplied context argument.
@@ -26,6 +44,13 @@
 typedef void Visitor(void *addr, void *arg);
 
 /*
+ * Like a Visitor, but passes root specific information such as the
+ * containing thread id and the root type.  In cases where a root is
+ * not specific to a thread, 0, an invalid thread id is provided.
+ */
+typedef void RootVisitor(void *addr, u4 threadId, RootType type, void *arg);
+
+/*
  * Visits references in an object.
  */
 void dvmVisitObject(Visitor *visitor, Object *obj, void *arg);
@@ -33,6 +58,6 @@ void dvmVisitObject(Visitor *visitor, Object *obj, void *arg);
 /*
  * Visits references in the root set.
  */
-void dvmVisitRoots(Visitor *visitor, void *arg);
+void dvmVisitRoots(RootVisitor *visitor, void *arg);
 
 #endif /* _DALVIK_ALLOC_VISIT */
diff --git a/vm/hprof/Hprof.c b/vm/hprof/Hprof.c
index 28f81c778..5cd5136c2 100644
--- a/vm/hprof/Hprof.c
+++ b/vm/hprof/Hprof.c
@@ -21,7 +21,10 @@
  * heap, and some analysis tools require that the class and string data
  * appear first.
  */
+
 #include "Hprof.h"
+#include "alloc/HeapInternal.h"
+#include "alloc/Visit.h"
 
 #include <string.h>
 #include <unistd.h>
@@ -30,7 +33,6 @@
 #include <sys/time.h>
 #include <time.h>
 
-
 #define kHeadSuffix "-hptemp"
 
 hprof_context_t *
@@ -183,3 +185,86 @@ hprofFreeContext(hprof_context_t *ctx)
     free(ctx->fileDataPtr);
     free(ctx);
 }
+
+/*
+ * Visitor invoked on every root reference.
+ */
+static void hprofRootVisitor(void *addr, u4 threadId, RootType type, void *arg)
+{
+    static const hprof_heap_tag_t xlate[] = {
+        HPROF_ROOT_UNKNOWN,
+        HPROF_ROOT_JNI_GLOBAL,
+        HPROF_ROOT_JNI_LOCAL,
+        HPROF_ROOT_JAVA_FRAME,
+        HPROF_ROOT_NATIVE_STACK,
+        HPROF_ROOT_STICKY_CLASS,
+        HPROF_ROOT_THREAD_BLOCK,
+        HPROF_ROOT_MONITOR_USED,
+        HPROF_ROOT_THREAD_OBJECT,
+        HPROF_ROOT_INTERNED_STRING,
+        HPROF_ROOT_FINALIZING,
+        HPROF_ROOT_DEBUGGER,
+        HPROF_ROOT_REFERENCE_CLEANUP,
+        HPROF_ROOT_VM_INTERNAL,
+        HPROF_ROOT_JNI_MONITOR,
+    };
+    hprof_context_t *ctx;
+
+    assert(arg != NULL);
+    assert(type < NELEM(xlate));
+    ctx = arg;
+    ctx->gcScanState = xlate[type];
+    ctx->gcThreadSerialNumber = threadId;
+    hprofMarkRootObject(ctx, addr, 0);
+    ctx->gcScanState = 0;
+    ctx->gcThreadSerialNumber = 0;
+}
+
+/*
+ * Visitor invoked on every heap object.
+ */
+static void hprofBitmapCallback(void *ptr, void *arg)
+{
+    Object *obj;
+    hprof_context_t *ctx;
+
+    assert(ptr != NULL);
+    assert(arg != NULL);
+    obj = ptr;
+    ctx = arg;
+    hprofDumpHeapObject(ctx, obj);
+}
+
+/*
+ * Walk the roots and heap writing heap information to the specified
+ * file.
+ *
+ * If "fd" is >= 0, the output will be written to that file descriptor.
+ * Otherwise, "fileName" is used to create an output file.
+ *
+ * If "directToDdms" is set, the other arguments are ignored, and data is
+ * sent directly to DDMS.
+ *
+ * Returns 0 on success, or an error code on failure.
+ */
+int hprofDumpHeap(const char* fileName, int fd, bool directToDdms)
+{
+    hprof_context_t *ctx;
+    int success;
+
+    assert(fileName != NULL);
+    dvmLockHeap();
+    dvmSuspendAllThreads(SUSPEND_FOR_HPROF);
+    ctx = hprofStartup(fileName, fd, directToDdms);
+    if (ctx == NULL) {
+        return -1;
+    }
+    dvmVisitRoots(hprofRootVisitor, ctx);
+    dvmHeapBitmapWalk(dvmHeapSourceGetLiveBits(), hprofBitmapCallback, ctx);
+    hprofFinishHeapDump(ctx);
+//TODO: write a HEAP_SUMMARY record
+    success = hprofShutdown(ctx) ? 0 : -1;
+    dvmSuspendAllThreads(SUSPEND_FOR_HPROF);
+    dvmUnlockHeap();
+    return success;
+}
diff --git a/vm/hprof/Hprof.h b/vm/hprof/Hprof.h
index 18f410253..f95e2e193 100644
--- a/vm/hprof/Hprof.h
+++ b/vm/hprof/Hprof.h
@@ -251,13 +251,9 @@ bool hprofShutdown(hprof_context_t *ctx);
 void hprofFreeContext(hprof_context_t *ctx);
 
 /*
- * Heap.c functions
- *
- * The contents of the hprof directory have no knowledge of
- * the heap implementation; these functions require heap knowledge,
- * so they are implemented in Heap.c.
+ * HprofVisit.c functions
  */
+
 int hprofDumpHeap(const char* fileName, int fd, bool directToDdms);
-void dvmHeapSetHprofGcScanState(hprof_heap_tag_t state, u4 threadSerialNumber);
 
 #endif  // _DALVIK_HPROF_HPROF
diff --git a/vm/native/dalvik_system_VMDebug.c b/vm/native/dalvik_system_VMDebug.c
index b9f3610a7..80e97e083 100644
--- a/vm/native/dalvik_system_VMDebug.c
+++ b/vm/native/dalvik_system_VMDebug.c
@@ -19,6 +19,7 @@
  */
 #include "Dalvik.h"
 #include "native/InternalNativePriv.h"
+#include "hprof/Hprof.h"
 
 #include <string.h>
 #include <unistd.h>