From 991d8b178b74028040fc08f78c45a2735d6b0a0e Mon Sep 17 00:00:00 2001
From: Geza Lore <gezalore@gmail.com>
Date: Wed, 8 Apr 2020 22:54:35 +0100
Subject: [PATCH] Fix FST tracing performance by removing std::map from hot
 path. (#2244)

This patch eliminates a major piece of inefficiency in FST tracing
support, by using an array to lookup fstHandle values corresponding
to trace codes, instead of a tree based std::map. With this change, FST
tracing is now only about 3x slower than VCD tracing. We do require
more memory to store the symbol lookup table, but the size of that is
still small, for the speed benefit.
---
 Changes                     |  2 ++
 include/verilated_fst_c.cpp | 18 +++++++++++++++++-
 include/verilated_fst_c.h   | 17 ++++++++---------
 3 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/Changes b/Changes
index c97291111..1860b60e3 100644
--- a/Changes
+++ b/Changes
@@ -15,6 +15,8 @@ The contributors that suggested a given feature are shown in []. Thanks!
 
 ****  Add error if use SystemC 2.2 and earlier (pre-2011) as is deprecated.
 
+****  Improve FST dump performance, #2244. [Geza Lore]
+
 
 * Verilator 4.032 2020-04-04
 
diff --git a/include/verilated_fst_c.cpp b/include/verilated_fst_c.cpp
index b0776e468..360e9b811 100644
--- a/include/verilated_fst_c.cpp
+++ b/include/verilated_fst_c.cpp
@@ -77,10 +77,16 @@ VerilatedFst::VerilatedFst(void* fst)
     : m_fst(fst)
     , m_fullDump(true)
     , m_nextCode(1)
-    , m_scopeEscape('.') {
+    , m_scopeEscape('.')
+    , m_symbolp(NULL) {
     m_valueStrBuffer.reserve(64 + 1);  // Need enough room for quad
 }
 
+VerilatedFst::~VerilatedFst() {
+    if (m_fst) fstWriterClose(m_fst);
+    if (m_symbolp) VL_DO_CLEAR(delete[] m_symbolp, m_symbolp = NULL);
+}
+
 void VerilatedFst::open(const char* filename) VL_MT_UNSAFE {
     m_assertOne.check();
     m_fst = fstWriterCreate(filename, 1);
@@ -104,6 +110,16 @@ void VerilatedFst::open(const char* filename) VL_MT_UNSAFE {
         fstWriterSetUpscope(m_fst);
         it = m_curScope.erase(it);
     }
+
+    // convert m_code2symbol into an array for fast lookup
+    if (!m_symbolp) {
+        m_symbolp = new fstHandle[m_nextCode + 10];
+        for (Code2SymbolType::iterator it = m_code2symbol.begin(); it != m_code2symbol.end();
+             ++it) {
+            m_symbolp[it->first] = it->second;
+        }
+    }
+    m_code2symbol.clear();
 }
 
 void VerilatedFst::module(const std::string& name) { m_module = name; }
diff --git a/include/verilated_fst_c.h b/include/verilated_fst_c.h
index bd7e6a830..60516eb25 100644
--- a/include/verilated_fst_c.h
+++ b/include/verilated_fst_c.h
@@ -55,6 +55,7 @@ private:
     Code2SymbolType m_code2symbol;
     Local2FstDtype m_local2fstdtype;
     std::list<std::string> m_curScope;
+    fstHandle* m_symbolp;  ///< same as m_code2symbol, but as an array
     // CONSTRUCTORS
     VL_UNCOPYABLE(VerilatedFst);
     void declSymbol(vluint32_t code, const char* name,
@@ -65,9 +66,7 @@ private:
 
 public:
     explicit VerilatedFst(void* fst = NULL);
-    ~VerilatedFst() {
-        if (m_fst == NULL) { fstWriterClose(m_fst); }
-    }
+    ~VerilatedFst();
     void changeThread() { m_assertOne.changeThread(); }
     bool isOpen() const { return m_fst != NULL; }
     void open(const char* filename) VL_MT_UNSAFE;
@@ -140,24 +139,24 @@ public:
 
     /// Inside dumping routines, dump one signal if it has changed
     void chgBit(vluint32_t code, const vluint32_t newval) {
-        fstWriterEmitValueChange(m_fst, m_code2symbol[code], newval ? "1" : "0");
+        fstWriterEmitValueChange(m_fst, m_symbolp[code], newval ? "1" : "0");
     }
     void chgBus(vluint32_t code, const vluint32_t newval, int bits) {
-        fstWriterEmitValueChange32(m_fst, m_code2symbol[code], bits, newval);
+        fstWriterEmitValueChange32(m_fst, m_symbolp[code], bits, newval);
     }
     void chgDouble(vluint32_t code, const double newval) {
         double val = newval;
-        fstWriterEmitValueChange(m_fst, m_code2symbol[code], &val);
+        fstWriterEmitValueChange(m_fst, m_symbolp[code], &val);
     }
     void chgFloat(vluint32_t code, const float newval) {
         double val = (double)newval;
-        fstWriterEmitValueChange(m_fst, m_code2symbol[code], &val);
+        fstWriterEmitValueChange(m_fst, m_symbolp[code], &val);
     }
     void chgQuad(vluint32_t code, const vluint64_t newval, int bits) {
-        fstWriterEmitValueChange64(m_fst, m_code2symbol[code], bits, newval);
+        fstWriterEmitValueChange64(m_fst, m_symbolp[code], bits, newval);
     }
     void chgArray(vluint32_t code, const vluint32_t* newval, int bits) {
-        fstWriterEmitValueChangeVec32(m_fst, m_code2symbol[code], bits, newval);
+        fstWriterEmitValueChangeVec32(m_fst, m_symbolp[code], bits, newval);
     }
 
     void fullBit(vluint32_t code, const vluint32_t newval) { chgBit(code, newval); }