[DynInst_API:] [PATCH] Use const string& parameters for stats


Date: Mon, 27 Jan 2014 18:30:25 -0800
From: Josh Stone <jistone@xxxxxxxxxx>
Subject: [DynInst_API:] [PATCH] Use const string& parameters for stats
The stats functions don't need mutable parameters, so a const& suffices
just fine.  Also convert all the fixed string names of counters and
timers from string literals to std::string in dyninstAPI and parseAPI.

Even when stats were disabled, there was a fair amount of time spent
just preparing the string object for stats calls.  The optimization of
this patch shows 5-10% reduction in parse time on large binaries.
---
 common/src/stats.C               | 21 ++++++++----------
 common/src/stats.h               | 14 ++++++------
 dyninstAPI/src/debug.C           | 24 +++++++++++++++++++++
 dyninstAPI/src/debug.h           | 46 ++++++++++++++++++++--------------------
 parseAPI/h/CodeSource.h          | 12 +++++------
 parseAPI/h/SymLiteCodeSource.h   |  6 +++---
 parseAPI/src/SymLiteCodeSource.C |  6 +++---
 parseAPI/src/SymtabCodeSource.C  |  6 +++---
 parseAPI/src/debug_parse.C       | 17 ++++++++++++++-
 parseAPI/src/debug_parse.h       | 22 +++++++++----------
 10 files changed, 105 insertions(+), 69 deletions(-)

diff --git a/common/src/stats.C b/common/src/stats.C
index 3bce10e6dfe7..e75e41e87762 100644
--- a/common/src/stats.C
+++ b/common/src/stats.C
@@ -77,17 +77,14 @@ StatContainer::StatContainer()
 }
 
 Statistic*
-StatContainer::operator[](std::string &name) 
+StatContainer::operator[](const std::string &name) 
 {
-    if (stats_.find(name) != stats_.end()) {
-        return stats_[name];
-    } else {
-        return NULL;
-    }
+    auto const& it = stats_.find(name);
+    return (it != stats_.end()) ? it->second : NULL;
 }
 
 void
-StatContainer::add(std::string name, StatType type)
+StatContainer::add(const std::string& name, StatType type)
 {
     Statistic *s = NULL;
 
@@ -107,32 +104,32 @@ StatContainer::add(std::string name, StatType type)
     stats_[name] = s;
 }
 
-void StatContainer::startTimer(std::string name) 
+void StatContainer::startTimer(const std::string& name) 
 {
     TimeStatistic *timer = dynamic_cast<TimeStatistic *>(stats_[name]);
     if (!timer) return;
     timer->start();
 }
 
-void StatContainer::stopTimer(std::string name) {
+void StatContainer::stopTimer(const std::string& name) {
     TimeStatistic *timer = dynamic_cast<TimeStatistic *>(stats_[name]);
     if (!timer) return;
     timer->stop();
 }
 
-void StatContainer::incrementCounter(std::string name) {
+void StatContainer::incrementCounter(const std::string& name) {
     CntStatistic *counter = dynamic_cast<CntStatistic *>(stats_[name]);
     if (!counter) return;
     (*counter)++;
 }
 
-void StatContainer::decrementCounter(std::string name) {
+void StatContainer::decrementCounter(const std::string& name) {
     CntStatistic *counter = dynamic_cast<CntStatistic *>(stats_[name]);
     if (!counter) return;
     (*counter)--;
 }
 
-void StatContainer::addCounter(std::string name, int val) {
+void StatContainer::addCounter(const std::string& name, int val) {
     CntStatistic *counter = dynamic_cast<CntStatistic *>(stats_[name]);
     if (!counter) return;
     (*counter) += val;
diff --git a/common/src/stats.h b/common/src/stats.h
index ba4fe93cb42e..69814ca309ca 100644
--- a/common/src/stats.h
+++ b/common/src/stats.h
@@ -171,7 +171,7 @@ class StatContainer {
      * This operator may return null if the named statistic does
      * not exist.
      */
-    COMMON_EXPORT Statistic * operator[](std::string &);
+    COMMON_EXPORT Statistic * operator[](const std::string &);
     COMMON_EXPORT Statistic * operator[](const char *s) {
        std::string namestr(s);
        return (*this)[namestr];
@@ -180,7 +180,7 @@ class StatContainer {
     // Create a new statistic of the given type indexed by name.
     // **This will replace any existing stat with the same index
     //   within this container**
-    COMMON_EXPORT void add(std::string name, StatType type);
+    COMMON_EXPORT void add(const std::string& name, StatType type);
 
     // Access all of the existing statistics
     COMMON_EXPORT dyn_hash_map< std::string, Statistic * > &
@@ -188,11 +188,11 @@ class StatContainer {
 
     // And some pass-through methods, encapsulated for
     // ease of use
-    COMMON_EXPORT void startTimer(std::string);
-    COMMON_EXPORT void stopTimer(std::string);
-    COMMON_EXPORT void incrementCounter(std::string);
-    COMMON_EXPORT void decrementCounter(std::string);
-    COMMON_EXPORT void addCounter(std::string, int);
+    COMMON_EXPORT void startTimer(const std::string&);
+    COMMON_EXPORT void stopTimer(const std::string&);
+    COMMON_EXPORT void incrementCounter(const std::string&);
+    COMMON_EXPORT void decrementCounter(const std::string&);
+    COMMON_EXPORT void addCounter(const std::string&, int);
 
  private:
     dyn_hash_map< std::string, Statistic * > stats_;
diff --git a/dyninstAPI/src/debug.C b/dyninstAPI/src/debug.C
index 6a771c235aab..8f9aa16b2543 100644
--- a/dyninstAPI/src/debug.C
+++ b/dyninstAPI/src/debug.C
@@ -702,6 +702,30 @@ StatContainer stats_ptrace;
 StatContainer stats_parse;
 StatContainer stats_codegen;
 
+const std::string INST_GENERATE_TIMER("instGenerateTimer");
+const std::string INST_INSTALL_TIMER("instInstallTimer");
+const std::string INST_LINK_TIMER("instLinkTimer");
+const std::string INST_REMOVE_TIMER("instRemoveTimer");
+const std::string INST_GENERATE_COUNTER("instGenerateCounter");
+const std::string INST_INSTALL_COUNTER("instInstallCounter");
+const std::string INST_LINK_COUNTER("instLinkCounter");
+const std::string INST_REMOVE_COUNTER("instRemoveCounter");
+
+const std::string PTRACE_WRITE_TIMER("ptraceWriteTimer");
+const std::string PTRACE_WRITE_COUNTER("ptraceWriteCounter");
+const std::string PTRACE_WRITE_AMOUNT("ptraceWriteAmountCounter");
+const std::string PTRACE_READ_TIMER("ptraceReadTimer");
+const std::string PTRACE_READ_COUNTER("ptraceReadCounter");
+const std::string PTRACE_READ_AMOUNT("ptraceReadAmountCounter");
+
+const std::string PARSE_SYMTAB_TIMER("parseSymtabTimer");
+const std::string PARSE_ANALYZE_TIMER("parseAnalyzeTimer");
+
+const std::string CODEGEN_AST_TIMER("codegenAstTimer");
+const std::string CODEGEN_AST_COUNTER("codegenAstCounter");
+const std::string CODEGEN_REGISTER_TIMER("codegenRegisterTimer");
+const std::string CODEGEN_LIVENESS_TIMER("codegenLivenessTimer");
+
 TimeStatistic running_time;
 
 bool have_stats = 0;
diff --git a/dyninstAPI/src/debug.h b/dyninstAPI/src/debug.h
index 4ef7ed7d248a..586b66021d3b 100644
--- a/dyninstAPI/src/debug.h
+++ b/dyninstAPI/src/debug.h
@@ -87,29 +87,29 @@ extern StatContainer stats_ptrace;
 extern StatContainer stats_parse;
 extern StatContainer stats_codegen;
 
-#define INST_GENERATE_TIMER "instGenerateTimer"
-#define INST_INSTALL_TIMER "instInstallTimer"
-#define INST_LINK_TIMER "instLinkTimer"
-#define INST_REMOVE_TIMER "instRemoveTimer"
-#define INST_GENERATE_COUNTER "instGenerateCounter"
-#define INST_INSTALL_COUNTER "instInstallCounter"
-#define INST_LINK_COUNTER "instLinkCounter"
-#define INST_REMOVE_COUNTER "instRemoveCounter"
-
-#define PTRACE_WRITE_TIMER "ptraceWriteTimer"
-#define PTRACE_WRITE_COUNTER "ptraceWriteCounter"
-#define PTRACE_WRITE_AMOUNT "ptraceWriteAmountCounter"
-#define PTRACE_READ_TIMER "ptraceReadTimer"
-#define PTRACE_READ_COUNTER "ptraceReadCounter"
-#define PTRACE_READ_AMOUNT "ptraceReadAmountCounter"
-
-#define PARSE_SYMTAB_TIMER "parseSymtabTimer"
-#define PARSE_ANALYZE_TIMER "parseAnalyzeTimer"
-
-#define CODEGEN_AST_TIMER "codegenAstTimer"
-#define CODEGEN_AST_COUNTER "codegenAstCounter"
-#define CODEGEN_REGISTER_TIMER "codegenRegisterTimer"
-#define CODEGEN_LIVENESS_TIMER "codegenLivenessTimer"
+extern const std::string INST_GENERATE_TIMER;
+extern const std::string INST_INSTALL_TIMER;
+extern const std::string INST_LINK_TIMER;
+extern const std::string INST_REMOVE_TIMER;
+extern const std::string INST_GENERATE_COUNTER;
+extern const std::string INST_INSTALL_COUNTER;
+extern const std::string INST_LINK_COUNTER;
+extern const std::string INST_REMOVE_COUNTER;
+
+extern const std::string PTRACE_WRITE_TIMER;
+extern const std::string PTRACE_WRITE_COUNTER;
+extern const std::string PTRACE_WRITE_AMOUNT;
+extern const std::string PTRACE_READ_TIMER;
+extern const std::string PTRACE_READ_COUNTER;
+extern const std::string PTRACE_READ_AMOUNT;
+
+extern const std::string PARSE_SYMTAB_TIMER;
+extern const std::string PARSE_ANALYZE_TIMER;
+
+extern const std::string CODEGEN_AST_TIMER;
+extern const std::string CODEGEN_AST_COUNTER;
+extern const std::string CODEGEN_REGISTER_TIMER;
+extern const std::string CODEGEN_LIVENESS_TIMER;
 
 // C++ prototypes
 #define signal_cerr       if (dyn_debug_signal) cerr
diff --git a/parseAPI/h/CodeSource.h b/parseAPI/h/CodeSource.h
index 4e9d3384c1b9..899f22b4acb1 100644
--- a/parseAPI/h/CodeSource.h
+++ b/parseAPI/h/CodeSource.h
@@ -171,9 +171,9 @@ class PARSER_EXPORT CodeSource : public Dyninst::InstructionSource {
     virtual bool have_stats() const { return false; }
 
     // manage statistics
-    virtual void incrementCounter(std::string /*name*/) const { return; } 
-    virtual void addCounter(std::string /*name*/, int /*num*/) const { return; }
-    virtual void decrementCounter(std::string /*name*/) const { return; }
+    virtual void incrementCounter(const std::string& /*name*/) const { return; } 
+    virtual void addCounter(const std::string& /*name*/, int /*num*/) const { return; }
+    virtual void decrementCounter(const std::string& /*name*/) const { return; }
     
  protected:
     CodeSource() : _regions_overlap(false),
@@ -277,9 +277,9 @@ class PARSER_EXPORT SymtabCodeSource : public CodeSource {
     bool have_stats() const { return _have_stats; }
 
     // manage statistics
-    void incrementCounter(std::string name) const;
-    void addCounter(std::string name, int num) const; 
-    void decrementCounter(std::string name) const;
+    void incrementCounter(const std::string& name) const;
+    void addCounter(const std::string& name, int num) const; 
+    void decrementCounter(const std::string& name) const;
 
  private:
     void init(hint_filt *, bool);
diff --git a/parseAPI/h/SymLiteCodeSource.h b/parseAPI/h/SymLiteCodeSource.h
index 9574798945d4..89727861f1c4 100644
--- a/parseAPI/h/SymLiteCodeSource.h
+++ b/parseAPI/h/SymLiteCodeSource.h
@@ -124,9 +124,9 @@ class SymReaderCodeSource : public CodeSource {
     PARSER_EXPORT bool have_stats() const { return _have_stats; }
 
     // manage statistics
-    void incrementCounter(std::string name) const;
-    void addCounter(std::string name, int num) const; 
-    void decrementCounter(std::string name) const;
+    void incrementCounter(const std::string& name) const;
+    void addCounter(const std::string& name, int num) const; 
+    void decrementCounter(const std::string& name) const;
 
  private:
 
diff --git a/parseAPI/src/SymLiteCodeSource.C b/parseAPI/src/SymLiteCodeSource.C
index 111a961438d0..eaa36e7eae24 100644
--- a/parseAPI/src/SymLiteCodeSource.C
+++ b/parseAPI/src/SymLiteCodeSource.C
@@ -344,7 +344,7 @@ SymReaderCodeSource::print_stats() const {
 }
 
 void
-SymReaderCodeSource::incrementCounter(std::string name) const
+SymReaderCodeSource::incrementCounter(const std::string& name) const
 {
     if (_have_stats) {
         stats_parse->incrementCounter(name);
@@ -352,7 +352,7 @@ SymReaderCodeSource::incrementCounter(std::string name) const
 }
 
 void 
-SymReaderCodeSource::addCounter(std::string name, int num) const
+SymReaderCodeSource::addCounter(const std::string& name, int num) const
 {
     if (_have_stats) {
         stats_parse->addCounter(name, num);
@@ -360,7 +360,7 @@ SymReaderCodeSource::addCounter(std::string name, int num) const
 }
 
 void
-SymReaderCodeSource::decrementCounter(std::string name) const
+SymReaderCodeSource::decrementCounter(const std::string& name) const
 {
     if (_have_stats) {
         stats_parse->decrementCounter(name);
diff --git a/parseAPI/src/SymtabCodeSource.C b/parseAPI/src/SymtabCodeSource.C
index c800eaf1e7b7..d54cc7de9dfc 100644
--- a/parseAPI/src/SymtabCodeSource.C
+++ b/parseAPI/src/SymtabCodeSource.C
@@ -313,7 +313,7 @@ SymtabCodeSource::print_stats() const {
 }
 
 void
-SymtabCodeSource::incrementCounter(std::string name) const
+SymtabCodeSource::incrementCounter(const std::string& name) const
 {
     if (_have_stats) {
         stats_parse->incrementCounter(name);
@@ -321,7 +321,7 @@ SymtabCodeSource::incrementCounter(std::string name) const
 }
 
 void 
-SymtabCodeSource::addCounter(std::string name, int num) const
+SymtabCodeSource::addCounter(const std::string& name, int num) const
 {
     if (_have_stats) {
         stats_parse->addCounter(name, num);
@@ -329,7 +329,7 @@ SymtabCodeSource::addCounter(std::string name, int num) const
 }
 
 void
-SymtabCodeSource::decrementCounter(std::string name) const
+SymtabCodeSource::decrementCounter(const std::string& name) const
 {
     if (_have_stats) {
         stats_parse->decrementCounter(name);
diff --git a/parseAPI/src/debug_parse.C b/parseAPI/src/debug_parse.C
index 2ac0d7c29f90..b2a51d6f86e6 100644
--- a/parseAPI/src/debug_parse.C
+++ b/parseAPI/src/debug_parse.C
@@ -85,7 +85,22 @@ int Dyninst::ParseAPI::malware_printf_int(const char *format, ...)
 
     return ret;
 }
-        
+
+const std::string PARSE_BLOCK_COUNT("parseBlockCount");
+const std::string PARSE_FUNCTION_COUNT("parseFunctionCount");
+const std::string PARSE_BLOCK_SIZE("parseBlockSize");
+
+const std::string PARSE_NORETURN_COUNT("parseNoReturnCount");
+const std::string PARSE_RETURN_COUNT("parseReturnCount");
+const std::string PARSE_UNKNOWN_COUNT("parseUnknownCount");
+
+const std::string PARSE_NORETURN_HEURISTIC("parseNoReturnHeuristicCount");
+
+const std::string PARSE_JUMPTABLE_COUNT("parseJumptableCount");
+const std::string PARSE_JUMPTABLE_FAIL("parseJumptableFail");
+const std::string PARSE_TAILCALL_COUNT("isTailcallCount");
+const std::string PARSE_TAILCALL_FAIL("isTailcallFail");
+
 #if defined(_MSC_VER)
 #pragma warning(pop)    
 #endif
diff --git a/parseAPI/src/debug_parse.h b/parseAPI/src/debug_parse.h
index 5c115fd160cd..a8c858acca66 100644
--- a/parseAPI/src/debug_parse.h
+++ b/parseAPI/src/debug_parse.h
@@ -56,19 +56,19 @@ namespace ParseAPI {
 }
 }
 
-#define PARSE_BLOCK_COUNT "parseBlockCount"
-#define PARSE_FUNCTION_COUNT "parseFunctionCount"
-#define PARSE_BLOCK_SIZE "parseBlockSize"
+extern const std::string PARSE_BLOCK_COUNT;
+extern const std::string PARSE_FUNCTION_COUNT;
+extern const std::string PARSE_BLOCK_SIZE;
 
-#define PARSE_NORETURN_COUNT "parseNoReturnCount"
-#define PARSE_RETURN_COUNT "parseReturnCount"
-#define PARSE_UNKNOWN_COUNT "parseUnknownCount"
+extern const std::string PARSE_NORETURN_COUNT;
+extern const std::string PARSE_RETURN_COUNT;
+extern const std::string PARSE_UNKNOWN_COUNT;
 
-#define PARSE_NORETURN_HEURISTIC "parseNoReturnHeuristicCount"
+extern const std::string PARSE_NORETURN_HEURISTIC;
 
-#define PARSE_JUMPTABLE_COUNT "parseJumptableCount"
-#define PARSE_JUMPTABLE_FAIL "parseJumptableFail"
-#define PARSE_TAILCALL_COUNT "isTailcallCount"
-#define PARSE_TAILCALL_FAIL "isTailcallFail"
+extern const std::string PARSE_JUMPTABLE_COUNT;
+extern const std::string PARSE_JUMPTABLE_FAIL;
+extern const std::string PARSE_TAILCALL_COUNT;
+extern const std::string PARSE_TAILCALL_FAIL;
 
 #endif
-- 
1.8.5.3

[← Prev in Thread] Current Thread [Next in Thread→]
  • [DynInst_API:] [PATCH] Use const string& parameters for stats, Josh Stone <=