Message system: group block repeats by name and pool the per-message data graph

indra/llmessage/llmessagetemplate.cpp

@@ -59,22 +59,6 @@ void LLMsgVarData::addData(const void *data, S32 size, EMsgVariableType type, S3
     }
 }
 
-void LLMsgData::addDataFast(char *blockname, char *varname, const void *data, S32 size, EMsgVariableType type, S32 data_size)
-{
-    // remember that if the blocknumber is > 0 then the number is appended to the name
-    char *namep = (char *)blockname;
-    LLMsgBlkData* block_data = mMemberBlocks[namep];
-    if (block_data->mBlockNumber)
-    {
-        namep += block_data->mBlockNumber;
-        block_data->addData(varname, data, size, type, data_size);
-    }
-    else
-    {
-        block_data->addData(varname, data, size, type, data_size);
-    }
-}
-
 // LLMessageVariable functions and friends
 
 std::ostream& operator<<(std::ostream& s, LLMessageVariable &msg)

indra/llmessage/llmessagetemplate.h

@@ -93,6 +93,7 @@ class LLMsgVarDataMap
     const_iterator end() const      { return mEntries.end(); }
     bool empty() const              { return mEntries.empty(); }
     size_t size() const             { return mEntries.size(); }
+    void clear()                    { mEntries.clear(); } // keeps capacity for reuse
 
     // Returns the entry for name, inserting an empty one keyed to name if
     // absent (std::map::operator[] semantics — repeated misses yield the
@@ -149,6 +150,22 @@ class LLMsgBlkData
         }
     }
 
+    // Reset identity and drop variable data for pooled reuse, keeping the
+    // variable-map's capacity. Frees any heap payloads first: clearing the
+    // vector destroys the LLMsgVarData entries with their trivial destructor,
+    // which does not own mData.
+    void reinit(char *name, S32 blocknum)
+    {
+        for (LLMsgVarData& var : mMemberVarData)
+        {
+            var.deleteData();
+        }
+        mMemberVarData.clear();
+        mName = name;
+        mBlockNumber = blocknum;
+        mTotalSize = -1;
+    }
+
     LLMsgVarData& addVariable(const char *name, EMsgVariableType type)
     {
         LLMsgVarData& entry = mMemberVarData[name];
@@ -169,31 +186,134 @@ class LLMsgBlkData
     S32                                 mTotalSize;
 };
 
+// One decoded or built message's block data. Each block name owns an ordered
+// list of its repeat instances; groups are kept in first-seen order, which is
+// the template's block order for both decode and build.
+//
+// This replaces an older std::map keyed by (canonical name pointer + repeat
+// index) pointer arithmetic. String-table names are contiguous 64-byte rows,
+// so name + i for i past ~64 walked into the adjacent name's storage; two
+// block names within range plus enough repeats aliased keys, overwriting (and
+// leaking) blocks and corrupting data. ImprovedTerseObjectUpdate-class
+// messages reach ~180 repeats after zerocode expansion, so this was reachable
+// from inbound traffic. Grouping by name sidesteps the synthesized key
+// entirely and drops the per-block map node.
 class LLMsgData
 {
 public:
-    LLMsgData(const char *name) : mTotalSize(-1)
+    struct BlockGroup
     {
-        mName = (char *)name;
+        char*                       mName;   // canonical (string-table) block name
+        std::vector<LLMsgBlkData*>  mBlocks; // repeat instances in order; owned
+    };
+    typedef std::vector<BlockGroup> msg_blk_data_map_t;
+
+    LLMsgData(const char *name) : mName((char *)name), mTotalSize(-1)
+    {
+        mMemberBlocks.reserve(8);
     }
     ~LLMsgData()
     {
-        for_each(mMemberBlocks.begin(), mMemberBlocks.end(), DeletePairedPointer());
-        mMemberBlocks.clear();
+        for (BlockGroup& group : mMemberBlocks)
+        {
+            for (LLMsgBlkData* blockp : group.mBlocks)
+            {
+                delete blockp;
+            }
+        }
+        for (LLMsgBlkData* blockp : mBlockPool)
+        {
+            delete blockp;
+        }
     }
 
+    // Reset for reuse on the next message: recycle every block into the free
+    // pool and clear the groups, keeping all the vector capacity. Variable
+    // data on the recycled blocks is freed lazily by allocBlock() on reuse
+    // (or by the destructor), so a decode after warmup allocates nothing.
+    void reset(const char *name = nullptr)
+    {
+        for (BlockGroup& group : mMemberBlocks)
+        {
+            for (LLMsgBlkData* blockp : group.mBlocks)
+            {
+                mBlockPool.push_back(blockp);
+            }
+        }
+        mMemberBlocks.clear(); // keeps capacity
+        mName = (char *)name;
+        mTotalSize = -1;
+    }
+
+    // Obtain a block for name+blocknum (from the pool, or freshly allocated)
+    // and append it as the next repeat of its group; returns it.
+    LLMsgBlkData* allocBlock(char *name, S32 blocknum)
+    {
+        LLMsgBlkData* blockp;
+        if (!mBlockPool.empty())
+        {
+            blockp = mBlockPool.back();
+            mBlockPool.pop_back();
+            blockp->reinit(name, blocknum);
+        }
+        else
+        {
+            blockp = new LLMsgBlkData(name, blocknum);
+        }
+        getOrAddGroup(name).mBlocks.push_back(blockp);
+        return blockp;
+    }
+
+    // Append blockp as the next repeat of its (canonical) block name.
     void addBlock(LLMsgBlkData *blockp)
     {
-        mMemberBlocks[blockp->mName] = blockp;
+        getOrAddGroup(blockp->mName).mBlocks.push_back(blockp);
     }
 
-    void addDataFast(char *blockname, char *varname, const void *data, S32 size, EMsgVariableType type, S32 data_size = -1);
+    bool empty() const { return mMemberBlocks.empty(); }
 
-public:
-    typedef std::map<char*, LLMsgBlkData*> msg_blk_data_map_t;
-    msg_blk_data_map_t                  mMemberBlocks;
+    // Repeat #blocknum of name, or nullptr if name or that index is absent.
+    LLMsgBlkData* getBlock(const char* name, S32 blocknum = 0) const
+    {
+        const BlockGroup* group = findGroup(name);
+        if (!group || blocknum < 0 || blocknum >= (S32)group->mBlocks.size())
+        {
+            return nullptr;
+        }
+        return group->mBlocks[blocknum];
+    }
+
+    msg_blk_data_map_t                  mMemberBlocks; // groups in template order
     char                                *mName;
     S32                                 mTotalSize;
+
+private:
+    std::vector<LLMsgBlkData*>          mBlockPool; // recycled blocks ready for reuse
+
+    BlockGroup& getOrAddGroup(char* name)
+    {
+        for (BlockGroup& group : mMemberBlocks)
+        {
+            if (group.mName == name)
+            {
+                return group;
+            }
+        }
+        mMemberBlocks.push_back(BlockGroup{name, {}});
+        return mMemberBlocks.back();
+    }
+
+    const BlockGroup* findGroup(const char* name) const
+    {
+        for (const BlockGroup& group : mMemberBlocks)
+        {
+            if (group.mName == name)
+            {
+                return &group;
+            }
+        }
+        return nullptr;
+    }
 };
 
 // LLMessage* classes store the template of messages

indra/llmessage/llsdmessagebuilder.cpp

@@ -117,8 +117,11 @@ void LLSDMessageBuilder::addBinaryData(
     S32 size)
 {
     std::vector<U8> v;
-    v.resize(size);
-    memcpy(&(v[0]), reinterpret_cast<const U8*>(data), size);
+    if (size > 0)
+    {
+        v.resize(size);
+        memcpy(v.data(), reinterpret_cast<const U8*>(data), size);
+    }
     (*mCurrentBlock)[varname] = v;
 }
 
@@ -239,41 +242,22 @@ U32 LLSDMessageBuilder::buildMessage(U8*, U32, U8)
 
 void LLSDMessageBuilder::copyFromMessageData(const LLMsgData& data)
 {
-    // copy the blocks
-    // counting variables used to encode multiple block info
-    S32 block_count = 0;
-    char* block_name = NULL;
-
-    // loop through msg blocks to loop through variables, totalling up size
-    // data and filling the new (send) message
-    LLMsgData::msg_blk_data_map_t::const_iterator iter =
-        data.mMemberBlocks.begin();
-    LLMsgData::msg_blk_data_map_t::const_iterator end =
-        data.mMemberBlocks.end();
-    for(; iter != end; ++iter)
+    // walk each block group in template order, then each repeat in order,
+    // starting a fresh block per repeat and converting its variables to LLSD
+    for (const LLMsgData::BlockGroup& group : data.mMemberBlocks)
     {
-        const LLMsgBlkData* mbci = iter->second;
-        if(!mbci) continue;
-
-        // do we need to encode a block code?
-        if (block_count == 0)
+        for (const LLMsgBlkData* mbci : group.mBlocks)
         {
-            block_count = mbci->mBlockNumber;
-            block_name = (char*)mbci->mName;
-        }
-
-        // counting down mutliple blocks
-        block_count--;
-
-        nextBlock(block_name);
+            nextBlock(group.mName);
 
-        // now loop through the variables
-        LLMsgBlkData::msg_var_data_map_t::const_iterator dit = mbci->mMemberVarData.begin();
-        LLMsgBlkData::msg_var_data_map_t::const_iterator dend = mbci->mMemberVarData.end();
-
-        for(; dit != dend; ++dit)
-        {
-            const LLMsgVarData& mvci = *dit;
+            for (const LLMsgVarData& mvci : mbci->mMemberVarData)
+            {
+            // a placeholder variable that was never filled has no payload at
+            // all (null data pointer); there is nothing to convert
+            if (mvci.getSize() < 0)
+            {
+                continue;
+            }
             const char* varname = mvci.getName();
 
             switch(mvci.getType())
@@ -284,14 +268,17 @@ void LLSDMessageBuilder::copyFromMessageData(const LLMsgData& data)
 
             case MVT_VARIABLE:
                 {
-                    const char end = ((const char*)mvci.getData())[mvci.getSize()-1]; // Ensure null terminated
+                    // an empty field stores no payload at all (getData() may
+                    // be null), so don't probe for a terminating NUL
+                    const S32 size = mvci.getSize();
+                    const char end = size > 0 ? ((const char*)mvci.getData())[size-1] : 1;
                     if (mvci.getDataSize() == 1 && end == 0)
                     {
                         addString(varname, (const char*)mvci.getData());
                     }
                     else
                     {
-                        addBinaryData(varname, mvci.getData(), mvci.getSize());
+                        addBinaryData(varname, mvci.getData(), size);
                     }
                     break;
                 }
@@ -392,6 +379,7 @@ void LLSDMessageBuilder::copyFromMessageData(const LLMsgData& data)
                 LL_WARNS() << "Unknown type in conversion of message to LLSD" << LL_ENDL;
                 break;
             }
+            }
         }
     }
 }