Cleanup the StackTrace interfaces and profile interfaces so they

are more efficient and reduce the number of new/delete calls

arch/alpha/stacktrace.cc:
    - Change the StackTrace code so that the class can more easily be
    cleaned out and reused to avoid extra allocations.
    - Allow trace() to accept a static instruction pointer so it can
    determine if the instruction is worth tracing.  This is moved from
    the CPU.
    - provide constants for special meaning PCs (user, console, unknown),
    instead of magic numbers
    - switch to using kernelSymtab instead of allSymtab which will be
    going away
    - if the stack adjustment doesn't make any sense, exit and push
    unknown so we don't get into an infinite loop or record garbage.
    - check to see if we've made too many iterations through the stack
    and panic to avoid an infinite loop
arch/alpha/stacktrace.hh:
    - Change the StackTrace code so that the class can more easily be
    cleaned out and reused to avoid extra allocations.
    - Allow trace() to accept a static instruction pointer so it can
    determine if the instruction is worth tracing.  This is moved from
    the CPU.
    - provide constants for special meaning PCs (user, console, unknown),
    instead of magic numbers
cpu/base.cc:
    only clear the profile if we have one
    include profile.hh here since base.hh doesn't do it anymore
cpu/base.hh:
    no need to include cpu/profile.hh here
cpu/profile.cc:
    use ProfileNode pointers instead of objects in the ChildList
    Consume a vector of addresses since that's really all we
    care about.
cpu/profile.hh:
    Keep pointers to ProfileNodes to reduce the size of these structures
    keep a StackTrace around so that we may reuse it.
    provide consume functions that use the new StackTrace trace interface
    one consume function is inline and tries to fastpath the no trace
    condition, it calls the outlined consume function if a trace is generated.
cpu/simple/cpu.cc:
    include cpu/profile.hh here since base.hh no longer does
    use the new FunctionProfile::consume interface
    (which contains the tracing functions)

--HG--
extra : convert_revision : 5a1d9265289a75f67a497b322926be1f8c2d8eb3

diff --git a/arch/alpha/stacktrace.cc b/arch/alpha/stacktrace.cc
index fdad9d6731cd4908dce367eada7dabc6357a8bdd..5a8df3d353e0434c9bceccb872b03dff89901149 100644 (file)
--- a/arch/alpha/stacktrace.cc
+++ b/arch/alpha/stacktrace.cc
@@ -103,25 +103,42 @@ ProcessInfo::name(Addr ksp) const
     return comm;
 }
 
-StackTrace::StackTrace(ExecContext *_xc, bool is_call)
-    : xc(_xc)
+StackTrace::StackTrace()
+    : xc(0), stack(64)
+{
+}
+
+StackTrace::StackTrace(ExecContext *_xc, StaticInstPtr<TheISA> inst)
+    : xc(0), stack(64)
 {
+    trace(_xc, inst);
+}
+
+StackTrace::~StackTrace()
+{
+}
+
+void
+StackTrace::trace(ExecContext *_xc, bool is_call)
+{
+    xc = _xc;
+
     bool usermode = (xc->regs.ipr[AlphaISA::IPR_DTB_CM] & 0x18) != 0;
 
     Addr pc = xc->regs.npc;
     bool kernel = xc->system->kernelStart <= pc && pc <= xc->system->kernelEnd;
 
     if (usermode) {
-        stack.push_back(1);
+        stack.push_back(user);
         return;
     }
 
     if (!kernel) {
-        stack.push_back(2);
+        stack.push_back(console);
         return;
     }
 
-    SymbolTable *symtab = xc->system->allSymtab;
+    SymbolTable *symtab = xc->system->kernelSymtab;
     Addr ksp = xc->regs.intRegFile[TheISA::StackPointerReg];
     Addr bottom = ksp & ~0x3fff;
     Addr addr;
@@ -151,10 +168,15 @@ StackTrace::StackTrace(ExecContext *_xc, bool is_call)
             if (!ra)
                 return;
 
+            if (size <= 0) {
+                stack.push_back(unknown);
+                return;
+            }
+
             pc = ra;
             ksp += size;
         } else {
-            stack.push_back(3);
+            stack.push_back(unknown);
             return;
         }
 
@@ -162,15 +184,14 @@ StackTrace::StackTrace(ExecContext *_xc, bool is_call)
             pc <= xc->system->kernelEnd;
         if (!kernel)
             return;
+
+        if (stack.size() >= 1000)
+            panic("unwinding too far");
     }
 
     panic("unwinding too far");
 }
 
-StackTrace::~StackTrace()
-{
-}
-
 bool
 StackTrace::isEntry(Addr addr)
 {
@@ -302,18 +323,18 @@ void
 StackTrace::dump()
 {
     StringWrap name(xc->cpu->name());
-    SymbolTable *symtab = xc->system->allSymtab;
+    SymbolTable *symtab = xc->system->kernelSymtab;
 
     DPRINTFN("------ Stack ------\n");
 
     string symbol;
     for (int i = 0, size = stack.size(); i < size; ++i) {
         Addr addr = stack[size - i - 1];
-        if (addr == 1)
+        if (addr == user)
             symbol = "user";
-        else if (addr == 2)
+        else if (addr == console)
             symbol = "console";
-        else if (addr == 3)
+        else if (addr == unknown)
             symbol = "unknown";
         else
             symtab->findSymbol(addr, symbol);

diff --git a/arch/alpha/stacktrace.hh b/arch/alpha/stacktrace.hh
index 5a4741eba00a7f91ed39628cbfaec691d6c35eb4..244e574b6261c310a94a469d3fb891a1f408d8e7 100644 (file)
--- a/arch/alpha/stacktrace.hh
+++ b/arch/alpha/stacktrace.hh
@@ -34,7 +34,6 @@
 
 class ExecContext;
 class StackTrace;
-class SymbolTable;
 
 class ProcessInfo
 {
@@ -67,13 +66,28 @@ class StackTrace
     bool decodeSave(MachInst inst, int &reg, int &disp);
     bool decodeStack(MachInst inst, int &disp);
 
+    void trace(ExecContext *xc, bool is_call);
+
   public:
-    StackTrace(ExecContext *xc, bool is_call);
+    StackTrace();
+    StackTrace(ExecContext *xc, StaticInstPtr<TheISA> inst);
     ~StackTrace();
 
+    void clear()
+    {
+        xc = 0;
+        stack.clear();
+    }
+
+    bool valid() const { return xc != NULL; }
+    bool trace(ExecContext *xc, StaticInstPtr<TheISA> inst);
+
   public:
     const std::vector<Addr> &getstack() const { return stack; }
-    static StackTrace *create(ExecContext *xc, StaticInstPtr<TheISA> inst);
+
+    static const int user = 1;
+    static const int console = 2;
+    static const int unknown = 3;
 
 #if TRACING_ON
   private:
@@ -87,13 +101,17 @@ class StackTrace
 #endif
 };
 
-inline StackTrace *
-StackTrace::create(ExecContext *xc, StaticInstPtr<TheISA> inst)
+inline bool
+StackTrace::trace(ExecContext *xc, StaticInstPtr<TheISA> inst)
 {
     if (!inst->isCall() && !inst->isReturn())
-        return NULL;
+        return false;
+
+    if (valid())
+        clear();
 
-    return new StackTrace(xc, !inst->isReturn());
+    trace(xc, !inst->isReturn());
+    return true;
 }
 
 #endif // __ARCH_ALPHA_STACKTRACE_HH__

diff --git a/cpu/base.cc b/cpu/base.cc
index a6e71c8086ce97aebfdef0471f39101724024840..8b94b85332a7c2177ac3b750bc87adf22a9f9ad8 100644 (file)
--- a/cpu/base.cc
+++ b/cpu/base.cc
@@ -36,6 +36,7 @@
 #include "base/output.hh"
 #include "cpu/base.hh"
 #include "cpu/exec_context.hh"
+#include "cpu/profile.hh"
 #include "cpu/sampler/sampler.hh"
 #include "sim/param.hh"
 #include "sim/sim_events.hh"
@@ -254,7 +255,8 @@ BaseCPU::takeOverFrom(BaseCPU *oldCPU)
     intstatus = oldCPU->intstatus;
 
     for (int i = 0; i < execContexts.size(); ++i)
-        execContexts[i]->profile->clear();
+        if (execContexts[i]->profile)
+            execContexts[i]->profile->clear();
 
     if (profileEvent)
         profileEvent->schedule(curTick);

diff --git a/cpu/base.hh b/cpu/base.hh
index 914d0698214a03220d305685f8683f7a9729e337..4a44ab804ac9e0486cd70c34935df9b9b8894acf 100644 (file)
--- a/cpu/base.hh
+++ b/cpu/base.hh
@@ -33,7 +33,6 @@
 
 #include "base/statistics.hh"
 #include "config/full_system.hh"
-#include "cpu/profile.hh"
 #include "cpu/sampler/sampler.hh"
 #include "sim/eventq.hh"
 #include "sim/sim_object.hh"

diff --git a/cpu/profile.cc b/cpu/profile.cc
index b17a3c74ef1b16f6bbf419843192ef048ff444da..f4aa81c2b72b32b2132371dafc078704efbdc6b6 100644 (file)
--- a/cpu/profile.cc
+++ b/cpu/profile.cc
@@ -47,8 +47,8 @@ ProfileNode::dump(const string &symbol, uint64_t id, const SymbolTable *symtab,
     ccprintf(os, "%#x %s %d ", id, symbol, count);
     ChildList::const_iterator i, end = children.end();
     for (i = children.begin(); i != end; ++i) {
-        const ProfileNode &node = i->second;
-        ccprintf(os, "%#x ", (intptr_t)&node);
+        const ProfileNode *node = i->second;
+        ccprintf(os, "%#x ", (intptr_t)node);
     }
 
     ccprintf(os, "\n");
@@ -65,8 +65,8 @@ ProfileNode::dump(const string &symbol, uint64_t id, const SymbolTable *symtab,
         else if (!symtab->findSymbol(addr, symbol))
             panic("could not find symbol for address %#x\n", addr);
 
-        const ProfileNode &node = i->second;
-        node.dump(symbol, (intptr_t)&node, symtab, os);
+        const ProfileNode *node = i->second;
+        node->dump(symbol, (intptr_t)node, symtab, os);
     }
 }
 
@@ -75,11 +75,8 @@ ProfileNode::clear()
 {
     count = 0;
     ChildList::iterator i, end = children.end();
-    for (i = children.begin(); i != end; ++i) {
-        ProfileNode &node = i->second;
-        node.clear();
-    }
-
+    for (i = children.begin(); i != end; ++i)
+        i->second->clear();
 }
 
 FunctionProfile::FunctionProfile(const SymbolTable *_symtab)
@@ -92,12 +89,16 @@ FunctionProfile::~FunctionProfile()
 }
 
 ProfileNode *
-FunctionProfile::consume(const StackTrace *trace)
+FunctionProfile::consume(const vector<Addr> &stack)
 {
-    const vector<Addr> &stack = trace->getstack();
     ProfileNode *current = &top;
-    for (int i = 0, size = stack.size(); i < size; ++i)
-        current = &current->children[stack[size - i - 1]];
+    for (int i = 0, size = stack.size(); i < size; ++i) {
+        ProfileNode *&ptr = current->children[stack[size - i - 1]];
+        if (ptr == NULL)
+            ptr = new ProfileNode;
+
+        current = ptr;
+    }
 
     return current;
 }

diff --git a/cpu/profile.hh b/cpu/profile.hh
index 9da170eb4eeec5bff323f7878270a0ac74adf58e..c795b8f4149c026d9bf5d047f92c2227f5363b4c 100644 (file)
--- a/cpu/profile.hh
+++ b/cpu/profile.hh
@@ -40,7 +40,7 @@ class ProfileNode
   private:
     friend class FunctionProfile;
 
-    typedef std::map<Addr, ProfileNode> ChildList;
+    typedef std::map<Addr, ProfileNode *> ChildList;
     ChildList children;
 
   public:
@@ -60,15 +60,26 @@ class FunctionProfile
     const SymbolTable *symtab;
     ProfileNode top;
     std::map<Addr, Counter> pc_count;
+    StackTrace trace;
 
   public:
     FunctionProfile(const SymbolTable *symtab);
     ~FunctionProfile();
 
-    ProfileNode *consume(const StackTrace *trace);
+    ProfileNode *consume(ExecContext *xc, StaticInstPtr<TheISA> inst);
+    ProfileNode *consume(const std::vector<Addr> &stack);
     void clear();
     void dump(ExecContext *xc, std::ostream &out) const;
     void sample(ProfileNode *node, Addr pc);
 };
 
+inline ProfileNode *
+FunctionProfile::consume(ExecContext *xc, StaticInstPtr<TheISA> inst)
+{
+    if (!trace.trace(xc, inst))
+        return NULL;
+    trace.dprintf();
+    return consume(trace.getstack());
+}
+
 #endif // __CPU_PROFILE_HH__

diff --git a/cpu/simple/cpu.cc b/cpu/simple/cpu.cc
index 8f7534e163f7e1b0e7255652cffad4280796db8a..862fe5b2c9890176c29f6f3b6697b64e6c59404c 100644 (file)
--- a/cpu/simple/cpu.cc
+++ b/cpu/simple/cpu.cc
@@ -46,6 +46,7 @@
 #include "cpu/base.hh"
 #include "cpu/exec_context.hh"
 #include "cpu/exetrace.hh"
+#include "cpu/profile.hh"
 #include "cpu/sampler/sampler.hh"
 #include "cpu/simple/cpu.hh"
 #include "cpu/smt.hh"
@@ -763,12 +764,7 @@ SimpleCPU::tick()
         if (xc->profile) {
             bool usermode = (xc->regs.ipr[AlphaISA::IPR_DTB_CM] & 0x18) != 0;
             xc->profilePC = usermode ? 1 : xc->regs.pc;
-            StackTrace *trace = StackTrace::create(xc, inst);
-            if (trace) {
-                xc->profileNode = xc->profile->consume(trace);
-                trace->dprintf();
-                delete trace;
-            }
+            xc->profileNode = xc->profile->consume(xc, inst);
         }
 #endif