Rework handling of modified ledger nodes:

* Track dirty nodes in a set
* Make flushDirty a member function
* Don't write deleted nodes
* Make sure new nodes are shareable
* Put new nodes in the treeNodeCache

src/ripple_app/consensus/LedgerConsensus.cpp

@@ -898,23 +898,23 @@ private:
             applyTransactions (set, newLCL, newLCL, failedTransactions, false);
             newLCL->updateSkipList ();
             newLCL->setClosed ();
-            boost::shared_ptr<SHAMap::NodeMap> acctNodes 
+            boost::shared_ptr<SHAMap::DirtySet> acctNodes
                 = newLCL->peekAccountStateMap ()->disarmDirty ();
-            boost::shared_ptr<SHAMap::NodeMap> txnNodes 
+            boost::shared_ptr<SHAMap::DirtySet> txnNodes
                 = newLCL->peekTransactionMap ()->disarmDirty ();
 
             // write out dirty nodes (temporarily done here)
             int fc;
 
-            while ((fc = SHAMap::flushDirty (*acctNodes, 256
-                , hotACCOUNT_NODE, newLCL->getLedgerSeq ())) > 0)
+            while ((fc = newLCL->peekAccountStateMap()->flushDirty (
+                *acctNodes, 256, hotACCOUNT_NODE, newLCL->getLedgerSeq ())) > 0)
             {
                 WriteLog (lsTRACE, LedgerConsensus) 
                     << "Flushed " << fc << " dirty state nodes";
             }
 
-            while ((fc = SHAMap::flushDirty (*txnNodes, 256
-                , hotTRANSACTION_NODE, newLCL->getLedgerSeq ())) > 0)
+            while ((fc = newLCL->peekTransactionMap()->flushDirty (
+                *txnNodes, 256, hotTRANSACTION_NODE, newLCL->getLedgerSeq ())) > 0)
             {
                 WriteLog (lsTRACE, LedgerConsensus) 
                     << "Flushed " << fc << " dirty transaction nodes";

src/ripple_app/ledger/Ledger.cpp

@@ -59,7 +59,8 @@ Ledger::Ledger (const RippleAddress& masterID, std::uint64_t startAmount)
 
     writeBack (lepCREATE, startAccount->getSLE ());
 
-    SHAMap::flushDirty (*mAccountStateMap->disarmDirty (), 256, hotACCOUNT_NODE, mLedgerSeq);
+    mAccountStateMap->flushDirty (*mAccountStateMap->disarmDirty (), 256,
+        hotACCOUNT_NODE, mLedgerSeq);
 
     initializeFees ();
 }

src/ripple_app/shamap/SHAMap.cpp

@@ -381,7 +381,7 @@ void SHAMap::returnNode (SHAMapTreeNode::pointer& node, bool modify)
             root = node;
 
         if (mDirtyNodes)
-            (*mDirtyNodes)[*node] = node;
+            mDirtyNodes->insert (*node);
     }
 }
 
@@ -389,7 +389,7 @@ void SHAMap::trackNewNode (SHAMapTreeNode::pointer& node)
 {
     assert (node->getSeq() == mSeq);
     if (mDirtyNodes)
-        (*mDirtyNodes)[*node] = node;
+        mDirtyNodes->insert (*node);
 }
 
 SHAMapTreeNode* SHAMap::firstBelow (SHAMapTreeNode* node)
@@ -1093,38 +1093,51 @@ bool SHAMap::fetchRoot (uint256 const& hash, SHAMapSyncFilter* filter)
     return true;
 }
 
+/** Begin saving dirty nodes to be written later */
 int SHAMap::armDirty ()
 {
-    // begin saving dirty nodes
-    mDirtyNodes = boost::make_shared< ripple::unordered_map<SHAMapNode, SHAMapTreeNode::pointer, SHAMapNode_hash> > ();
+    mDirtyNodes = boost::make_shared <DirtySet> ();
     return ++mSeq;
 }
 
-int SHAMap::flushDirty (NodeMap& map, int maxNodes, NodeObjectType t, std::uint32_t seq)
+/** Write all modified nodes to the node store */
+int SHAMap::flushDirty (DirtySet& set, int maxNodes, NodeObjectType t, std::uint32_t seq)
 {
     int flushed = 0;
     Serializer s;
 
-    for (NodeMap::iterator it = map.begin (); it != map.end (); it = map.erase (it))
+    ScopedWriteLockType sl (mLock);
+
+    for (DirtySet::iterator it = set.begin (); it != set.end (); it = set.erase (it))
     {
-        //      tLog(t == hotTRANSACTION_NODE, lsDEBUG) << "TX node write " << it->first;
-        //      tLog(t == hotACCOUNT_NODE, lsDEBUG) << "STATE node write " << it->first;
+        SHAMapTreeNode::pointer node = checkCacheNode (*it);
+
+        // Check if node was deleted
+        if (!node)
+            continue;
+
+        uint256 const nodeHash = node->getNodeHash();
+
         s.erase ();
-        it->second->addRaw (s, snfPREFIX);
+        node->addRaw (s, snfPREFIX);
 
 #ifdef BEAST_DEBUG
 
-        if (s.getSHA512Half () != it->second->getNodeHash ())
+        if (s.getSHA512Half () != nodeHash)
         {
-            WriteLog (lsFATAL, SHAMap) << * (it->second);
+            WriteLog (lsFATAL, SHAMap) << *node;
             WriteLog (lsFATAL, SHAMap) << beast::lexicalCast <std::string> (s.getDataLength ());
-            WriteLog (lsFATAL, SHAMap) << s.getSHA512Half () << " != " << it->second->getNodeHash ();
+            WriteLog (lsFATAL, SHAMap) << s.getSHA512Half () << " != " << nodeHash;
             assert (false);
         }
 
 #endif
 
-        getApp().getNodeStore ().store (t, seq, std::move (s.modData ()), it->second->getNodeHash ());
+        getApp().getNodeStore ().store (t, seq, std::move (s.modData ()), nodeHash);
+
+        // Put the canonical version into the SHAMap and the treeNodeCache
+        mTNByID.erase (*node);
+        fetchNodeExternal (*node, nodeHash);
 
         if (flushed++ >= maxNodes)
             return flushed;
@@ -1133,12 +1146,12 @@ int SHAMap::flushDirty (NodeMap& map, int maxNodes, NodeObjectType t, std::uint3
     return flushed;
 }
 
-boost::shared_ptr<SHAMap::NodeMap> SHAMap::disarmDirty ()
+/** Stop saving dirty nodes */
+boost::shared_ptr<SHAMap::DirtySet> SHAMap::disarmDirty ()
 {
-    // stop saving dirty nodes
     ScopedWriteLockType sl (mLock);
 
-    boost::shared_ptr<NodeMap> ret;
+    boost::shared_ptr<DirtySet> ret;
     ret.swap (mDirtyNodes);
     return ret;
 }

src/ripple_app/shamap/SHAMap.h

@@ -112,6 +112,7 @@ public:
     typedef std::pair<SHAMapItem::ref, SHAMapItem::ref> DeltaRef;
     typedef std::map<uint256, DeltaItem> Delta;
     typedef ripple::unordered_map<SHAMapNode, SHAMapTreeNode::pointer, SHAMapNode_hash> NodeMap;
+    typedef std::unordered_set<SHAMapNode, SHAMapNode_hash> DirtySet;
 
     typedef boost::shared_mutex LockType;
     typedef boost::shared_lock<LockType> ScopedReadLockType;
@@ -229,9 +230,9 @@ public:
     bool compare (SHAMap::ref otherMap, Delta & differences, int maxCount);
 
     int armDirty ();
-    static int flushDirty (NodeMap & dirtyMap, int maxNodes, NodeObjectType t,
+    int flushDirty (DirtySet & dirtySet, int maxNodes, NodeObjectType t,
                            std::uint32_t seq);
-    boost::shared_ptr<NodeMap> disarmDirty ();
+    boost::shared_ptr<DirtySet> disarmDirty ();
 
     void setSeq (std::uint32_t seq)
     {
@@ -347,7 +348,7 @@ private:
     std::uint32_t mSeq;
     std::uint32_t mLedgerSeq; // sequence number of ledger this is part of
     SyncUnorderedMapType< SHAMapNode, SHAMapTreeNode::pointer, SHAMapNode_hash > mTNByID;
-    boost::shared_ptr<NodeMap> mDirtyNodes;
+    boost::shared_ptr<DirtySet> mDirtyNodes;
     SHAMapTreeNode::pointer root;
     SHAMapState mState;
     SHAMapType mType;