Memory-conserving changes to SHAMapTreeNode and visitLeavesInternal.

src/ripple_app/consensus/LedgerConsensus.cpp

@@ -1307,8 +1307,8 @@ void LedgerConsensus::accept (SHAMap::ref set, LoadEvent::pointer)
         applyTransactions (set, newLCL, newLCL, failedTransactions, false);
         newLCL->updateSkipList ();
         newLCL->setClosed ();
-        boost::shared_ptr<SHAMap::DirtyMap> acctNodes = newLCL->peekAccountStateMap ()->disarmDirty ();
+        boost::shared_ptr<SHAMap::NodeMap> acctNodes = newLCL->peekAccountStateMap ()->disarmDirty ();
-        boost::shared_ptr<SHAMap::DirtyMap> txnNodes = newLCL->peekTransactionMap ()->disarmDirty ();
+        boost::shared_ptr<SHAMap::NodeMap> txnNodes = newLCL->peekTransactionMap ()->disarmDirty ();
 
         // write out dirty nodes (temporarily done here)
         int fc;

src/ripple_app/main/Application.cpp

@@ -472,6 +472,7 @@ public:
         m_ledgerMaster->tune (getConfig ().getSize (siLedgerSize), getConfig ().getSize (siLedgerAge));
         m_sleCache.setTargetSize (getConfig ().getSize (siSLECacheSize));
         m_sleCache.setTargetAge (getConfig ().getSize (siSLECacheAge));
+        SHAMap::setTreeCache (getConfig ().getSize (siTreeCacheSize), getConfig ().getSize (siTreeCacheAge));
 
 
         //----------------------------------------------------------------------

src/ripple_app/rpc/RPCHandler.cpp

@@ -2713,6 +2713,7 @@ Json::Value RPCHandler::doGetCounts (Json::Value params, Resource::Charge& loadT
     ret["AL_hit_rate"] = AcceptedLedger::getCacheHitRate ();
 
     ret["fullbelow_size"] = SHAMap::getFullBelowSize ();
+    ret["treenode_size"] = SHAMap::getTreeNodeSize ();
 
     std::string uptime;
     int s = UptimeTimer::getInstance ().getElapsedSeconds ();

src/ripple_app/shamap/SHAMap.cpp

@@ -23,6 +23,9 @@
 
 SETUP_LOG (SHAMap)
 
+TaggedCacheType< SHAMap::TNIndex, SHAMapTreeNode, UptimeTimerAdapter>
+    SHAMap::treeNodeCache ("TreeNodeCache", 65536, 60);
+
 SHAMap::~SHAMap ()
 {
     mState = smsInvalid;
@@ -73,6 +76,7 @@ SHAMap::SHAMap (SHAMapType t, uint32 seq)
     , mState (smsModifying)
     , mType (t)
 {
+    assert (mSeq != 0);
     if (t == smtSTATE)
         mTNByID.rehash (STATE_MAP_BUCKETS);
 
@@ -99,17 +103,37 @@ SHAMap::SHAMap (SHAMapType t, uint256 const& hash)
 
 SHAMap::pointer SHAMap::snapShot (bool isMutable)
 {
-    // Return a new SHAMap that is an immutable snapshot of this one
-    // Initially nodes are shared, but CoW is forced on both ledgers
-    ScopedLockType sl (mLock, __FILE__, __LINE__);
     SHAMap::pointer ret = boost::make_shared<SHAMap> (mType);
-    SHAMap& newMap = *ret;
+        SHAMap& newMap = *ret;
-    newMap.mSeq = ++mSeq;
-    newMap.mTNByID = mTNByID;
-    newMap.root = root;
 
-    if (!isMutable)
+    // Return a new SHAMap that is a snapshot of this one
-        newMap.mState = smsImmutable;
+    // Initially most nodes are shared and CoW is forced where needed
+    {
+        ScopedLockType sl (mLock, __FILE__, __LINE__);
+        newMap.mSeq = mSeq;
+        newMap.mTNByID = mTNByID;
+        newMap.root = root;
+
+        if (!isMutable)
+            newMap.mState = smsImmutable;
+
+        // If the existing map has any nodes it might modify, unshare them now
+        if (mState != smsImmutable)
+        {
+            BOOST_FOREACH(NodeMap::value_type& nodeIt, mTNByID)
+            {
+                if (nodeIt.second->getSeq() == mSeq)
+                { // We might modify this node, so duplicate it in the snapShot
+                    SHAMapTreeNode::pointer newNode = boost::make_shared<SHAMapTreeNode> (*nodeIt.second, mSeq);
+                    newMap.mTNByID[*newNode] = newNode;
+                    if (newNode->isRoot ())
+                        newMap.root = newNode;
+                }
+            }
+        }
+        else if (isMutable) // Need to unshare on changes to the snapshot
+            ++newMap.mSeq;
+    }
 
     return ret;
 }
@@ -312,7 +336,8 @@ SHAMapTreeNode* SHAMap::getNodePointerNT (const SHAMapNode& id, uint256 const& h
         if (filter->haveNode (id, hash, nodeData))
         {
             SHAMapTreeNode::pointer node = boost::make_shared<SHAMapTreeNode> (
-                                               boost::cref (id), boost::cref (nodeData), mSeq - 1, snfPREFIX, boost::cref (hash), true);
+                    boost::cref (id), boost::cref (nodeData), 0, snfPREFIX, boost::cref (hash), true);
+            canonicalize (hash, node);
             mTNByID[id] = node;
             filter->gotNode (true, id, hash, nodeData, node->getType ());
             return node.get ();
@@ -349,6 +374,7 @@ void SHAMap::returnNode (SHAMapTreeNode::pointer& node, bool modify)
 
 void SHAMap::trackNewNode (SHAMapTreeNode::pointer& node)
 {
+    assert (node->getSeq() == mSeq);
     if (mDirtyNodes)
         (*mDirtyNodes)[*node] = node;
 }
@@ -867,54 +893,61 @@ SHAMapTreeNode::pointer SHAMap::fetchNodeExternalNT (const SHAMapNode& id, uint2
     if (!getApp().running ())
         return ret;
 
-    // These are for diagnosing a crash on exit
+    ret = getCache (hash, id);
-    Application& app (getApp ());
+    if (ret)
-    NodeStore::Database& nodeStore (app.getNodeStore ());
+    { // The node was found in the TreeNodeCache
-    NodeObject::pointer obj (nodeStore.fetch (hash));
+        assert (ret->getSeq() == 0);
-
+        assert (id == *ret);
-    if (!obj)
+    }
-    {
+    else
-        //      WriteLog (lsTRACE, SHAMap) << "fetchNodeExternal: missing " << hash;
+    { // Check the back end
-        if (mLedgerSeq != 0)
+        NodeObject::pointer obj (getApp ().getNodeStore ().fetch (hash));
+        if (!obj)
         {
-            getApp().getOPs ().missingNodeInLedger (mLedgerSeq);
+            if (mLedgerSeq != 0)
-            mLedgerSeq = 0;
+            {
+                getApp().getOPs ().missingNodeInLedger (mLedgerSeq);
+                mLedgerSeq = 0;
+            }
+
+            return ret;
         }
 
-        return ret;
+        try
-    }
-
-    try
-    {
-        ret = boost::make_shared<SHAMapTreeNode> (id, obj->getData (), mSeq, snfPREFIX, hash, true);
-
-        if (id != *ret)
         {
-            WriteLog (lsFATAL, SHAMap) << "id:" << id << ", got:" << *ret;
+            ret = boost::make_shared<SHAMapTreeNode> (id, obj->getData (), 0, snfPREFIX, hash, true);
-            assert (false);
+
+            if (id != *ret)
+            {
+                WriteLog (lsFATAL, SHAMap) << "id:" << id << ", got:" << *ret;
+                assert (false);
+                return SHAMapTreeNode::pointer ();
+            }
+
+            if (ret->getNodeHash () != hash)
+            {
+                WriteLog (lsFATAL, SHAMap) << "Hashes don't match";
+                assert (false);
+                return SHAMapTreeNode::pointer ();
+            }
+
+            canonicalize (hash, ret);
+        }
+        catch (...)
+        {
+            WriteLog (lsWARNING, SHAMap) << "fetchNodeExternal gets an invalid node: " << hash;
             return SHAMapTreeNode::pointer ();
         }
-
-        if (ret->getNodeHash () != hash)
-        {
-            WriteLog (lsFATAL, SHAMap) << "Hashes don't match";
-            assert (false);
-            return SHAMapTreeNode::pointer ();
-        }
-
-        if (id.isRoot ())
-            mTNByID[id] = ret;
-        else if (!mTNByID.emplace (id, ret).second)
-            assert (false);
-
-        trackNewNode (ret);
-        return ret;
     }
-    catch (...)
+
+    if (id.isRoot ()) // it is legal to replace an existing root
     {
-        WriteLog (lsWARNING, SHAMap) << "fetchNodeExternal gets an invalid node: " << hash;
+        mTNByID[id] = ret;
-        return SHAMapTreeNode::pointer ();
+        root = ret;
     }
+    else if (!mTNByID.emplace (id, ret).second)
+        assert (false);
+    return ret;
 }
 
 bool SHAMap::fetchRoot (uint256 const& hash, SHAMapSyncFilter* filter)
@@ -936,7 +969,7 @@ bool SHAMap::fetchRoot (uint256 const& hash, SHAMapSyncFilter* filter)
     
     if (newRoot)
     {
-    	root = newRoot;
+        root = newRoot;
     }
     else
     {
@@ -962,12 +995,12 @@ int SHAMap::armDirty ()
     return ++mSeq;
 }
 
-int SHAMap::flushDirty (DirtyMap& map, int maxNodes, NodeObjectType t, uint32 seq)
+int SHAMap::flushDirty (NodeMap& map, int maxNodes, NodeObjectType t, uint32 seq)
 {
     int flushed = 0;
     Serializer s;
 
-    for (DirtyMap::iterator it = map.begin (); it != map.end (); it = map.erase (it))
+    for (NodeMap::iterator it = map.begin (); it != map.end (); it = map.erase (it))
     {
         //      tLog(t == hotTRANSACTION_NODE, lsDEBUG) << "TX node write " << it->first;
         //      tLog(t == hotACCOUNT_NODE, lsDEBUG) << "STATE node write " << it->first;
@@ -995,12 +1028,12 @@ int SHAMap::flushDirty (DirtyMap& map, int maxNodes, NodeObjectType t, uint32 se
     return flushed;
 }
 
-boost::shared_ptr<SHAMap::DirtyMap> SHAMap::disarmDirty ()
+boost::shared_ptr<SHAMap::NodeMap> SHAMap::disarmDirty ()
 {
     // stop saving dirty nodes
     ScopedLockType sl (mLock, __FILE__, __LINE__);
 
-    boost::shared_ptr<DirtyMap> ret;
+    boost::shared_ptr<NodeMap> ret;
     ret.swap (mDirtyNodes);
     return ret;
 }
@@ -1124,6 +1157,19 @@ void SHAMap::dump (bool hash)
 
 }
 
+SHAMapTreeNode::pointer SHAMap::getCache (uint256 const& hash, SHAMapNode const& id)
+{
+    SHAMapTreeNode::pointer ret = treeNodeCache.fetch (TNIndex (hash, id));
+    assert (!ret || !ret->getSeq());
+    return ret;
+}
+
+void SHAMap::canonicalize (uint256 const& hash, SHAMapTreeNode::pointer& node)
+{
+    assert (node->getSeq() == 0);
+    treeNodeCache.canonicalize (TNIndex (hash, *node), node);
+}
+
 //------------------------------------------------------------------------------
 
 class SHAMapTests : public UnitTest

src/ripple_app/shamap/SHAMap.h

@@ -40,7 +40,7 @@ public:
 
     typedef std::pair<SHAMapItem::pointer, SHAMapItem::pointer> DeltaItem;
     typedef std::map<uint256, DeltaItem> Delta;
-    typedef boost::unordered_map<SHAMapNode, SHAMapTreeNode::pointer> DirtyMap;
+    typedef boost::unordered_map<SHAMapNode, SHAMapTreeNode::pointer> NodeMap;
 
     typedef RippleRecursiveMutex LockType;
     typedef LockType::ScopedLockType ScopedLockType;
@@ -160,12 +160,13 @@ public:
     bool compare (SHAMap::ref otherMap, Delta & differences, int maxCount);
 
     int armDirty ();
-    static int flushDirty (DirtyMap & dirtyMap, int maxNodes, NodeObjectType t, uint32 seq);
+    static int flushDirty (NodeMap & dirtyMap, int maxNodes, NodeObjectType t, uint32 seq);
-    boost::shared_ptr<DirtyMap> disarmDirty ();
+    boost::shared_ptr<NodeMap> disarmDirty ();
 
     void setSeq (uint32 seq)
     {
         mSeq = seq;
+        assert (seq != 0);
     }
     uint32 getSeq ()
     {
@@ -199,18 +200,35 @@ public:
     std::list<fetchPackEntry_t> getFetchPack (SHAMap * have, bool includeLeaves, int max);
     void getFetchPack (SHAMap * have, bool includeLeaves, int max, FUNCTION_TYPE<void (const uint256&, const Blob&)>);
 
+    // tree node cache operations
+    static SHAMapTreeNode::pointer getCache (uint256 const& hash, SHAMapNode const& id);
+    static void canonicalize (uint256 const& hash, SHAMapTreeNode::pointer&);
+
     static int getFullBelowSize ()
     {
         return fullBelowCache.getSize ();
     }
+    static int getTreeNodeSize ()
+    {
+        return treeNodeCache.getCacheSize ();
+    }
     static void sweep ()
     {
         fullBelowCache.sweep ();
+        treeNodeCache.sweep ();
+    }
+    static void setTreeCache (int size, int age)
+    {
+        treeNodeCache.setTargetSize (size);
+        treeNodeCache.setTargetAge (age);
     }
 
 private:
     static KeyCache <uint256, UptimeTimerAdapter> fullBelowCache;
 
+    typedef std::pair<uint256, SHAMapNode> TNIndex;
+    static TaggedCacheType <TNIndex, SHAMapTreeNode, UptimeTimerAdapter> treeNodeCache;
+
     void dirtyUp (std::stack<SHAMapTreeNode::pointer>& stack, uint256 const & target, uint256 prevHash);
     std::stack<SHAMapTreeNode::pointer> getStack (uint256 const & id, bool include_nonmatching_leaf);
     SHAMapTreeNode::pointer walkTo (uint256 const & id, bool modify);
@@ -249,9 +267,9 @@ private:
 
     uint32 mSeq;
     uint32 mLedgerSeq; // sequence number of ledger this is part of
-    boost::unordered_map<SHAMapNode, SHAMapTreeNode::pointer> mTNByID;
+    NodeMap mTNByID;
 
-    boost::shared_ptr<DirtyMap> mDirtyNodes;
+    boost::shared_ptr<NodeMap> mDirtyNodes;
 
     SHAMapTreeNode::pointer root;
 

src/ripple_app/shamap/SHAMapSync.cpp

@@ -57,7 +57,9 @@ void SHAMap::visitLeavesInternal (FUNCTION_TYPE<void (SHAMapItem::ref item)>& fu
         while (pos < 16)
         {
             if (node->isEmptyBranch (pos))
-                ++pos;
+            {
+                ++pos; // move to next position
+            }
             else
             {
                 SHAMapTreeNode* child = getNodePointer (node->getChildNodeID (pos), node->getChildHash (pos));
@@ -70,18 +72,23 @@ void SHAMap::visitLeavesInternal (FUNCTION_TYPE<void (SHAMapItem::ref item)>& fu
                 else
                 {
                     if (pos != 15)
-                        stack.push (posPair (pos + 1, node)); // save next position
+                        stack.push (posPair (pos + 1, node)); // save next position to resume at
                     else
                         mTNByID.erase (*node); // don't need this inner node anymore
 
+                    // descend to the child's first position
                     node = child;
                     pos = 0;
                 }
             }
         }
 
+        // We are done with this inner node
+        mTNByID.erase (*node);
+
         if (stack.empty ())
             break;
+
         pos = stack.top ().first;
         node = stack.top ().second;
         stack.pop ();
@@ -369,7 +376,7 @@ SHAMapAddNode SHAMap::addKnownNode (const SHAMapNode& node, Blob const& rawNode,
             }
 
             SHAMapTreeNode::pointer newNode =
-                boost::make_shared<SHAMapTreeNode> (node, rawNode, mSeq - 1, snfWIRE, uZero, false);
+                boost::make_shared<SHAMapTreeNode> (node, rawNode, 0, snfWIRE, uZero, false);
 
             if (iNode->getChildHash (branch) != newNode->getNodeHash ())
             {
@@ -377,6 +384,8 @@ SHAMapAddNode SHAMap::addKnownNode (const SHAMapNode& node, Blob const& rawNode,
                 return SHAMapAddNode::invalid ();
             }
 
+            canonicalize (iNode->getChildHash (branch), newNode);
+
             if (filter)
             {
                 Serializer s;

src/ripple_app/shamap/SHAMapTreeNode.cpp

@@ -32,7 +32,12 @@ SHAMapTreeNode::SHAMapTreeNode (const SHAMapTreeNode& node, uint32 seq) : SHAMap
     mHash (node.mHash), mSeq (seq), mType (node.mType), mIsBranch (node.mIsBranch), mFullBelow (false)
 {
     if (node.mItem)
-        mItem = boost::make_shared<SHAMapItem> (*node.mItem);
+    {
+        if ((mSeq == 0) && (node.mSeq == 0))
+            mItem = node.mItem; // two immutable nodes can share an item
+        else
+            mItem = boost::make_shared<SHAMapItem> (*node.mItem);
+    }
     else
         memcpy (mHashes, node.mHashes, sizeof (mHashes));
 }
@@ -378,6 +383,7 @@ bool SHAMapTreeNode::setItem (SHAMapItem::ref i, TNType type)
     mType = type;
     mItem = i;
     assert (isLeaf ());
+    assert (mSeq != 0);
     return updateHash ();
 }
 
@@ -464,6 +470,7 @@ bool SHAMapTreeNode::setChildHash (int m, uint256 const& hash)
 {
     assert ((m >= 0) && (m < 16));
     assert (mType == tnINNER);
+    assert (mSeq != 0);
 
     if (mHashes[m] == hash)
         return false;
@@ -477,3 +484,4 @@ bool SHAMapTreeNode::setChildHash (int m, uint256 const& hash)
 
     return updateHash ();
 }
+

src/ripple_core/functional/Config.cpp

@@ -559,9 +559,12 @@ int Config::getSize (SizedItemName item)
         { siValidationsSize,    {   256,    256,    512,    1024,       1024    } },
         { siValidationsAge,     {   500,    500,    500,    500,        500     } },
 
-        { siNodeCacheSize,      {   8192,   65536,  262144, 512000,     0       } },
+        { siNodeCacheSize,      {   8192,   16384,  32768,  131072,     0       } },
         { siNodeCacheAge,       {   30,     60,     90,     120,        900     } },
 
+        { siTreeCacheSize,      {   8192,   65536,  131072, 131072,     0       } },
+        { siTreeCacheAge,       {   30,     60,     90,     120,        900     } },
+
         { siSLECacheSize,       {   4096,   8192,   16384,  65536,      0       } },
         { siSLECacheAge,        {   30,     60,     90,     120,        300     } },
 

src/ripple_core/functional/Config.h

@@ -64,6 +64,8 @@ enum SizedItemName
     siValidationsAge,
     siNodeCacheSize,
     siNodeCacheAge,
+    siTreeCacheSize,
+    siTreeCacheAge,
     siSLECacheSize,
     siSLECacheAge,
     siLedgerSize,