Cache CellId

ESM::Cell::getCellId() was allocating a string on every call. This caused functions dealing with cellIds to be unnecessarily expensive.

For example, World::moveObject spent almost as much time comparing CellIds as it did updating Bullet's AABB after the move. OpGetDistance was by far the most expensive script instruction because it has to compare cellIds.

The total cost of getCellId() relative to the frame loop was about 0.3%.

apps/openmw/mwstate/statemanagerimp.cpp

@@ -475,7 +475,7 @@ void MWState::StateManager::loadGame (const Character *character, const std::str
 
         MWWorld::Ptr ptr = MWMechanics::getPlayer();
 
-        ESM::CellId cellId = ptr.getCell()->getCell()->getCellId();
+        const ESM::CellId& cellId = ptr.getCell()->getCell()->getCellId();
 
         // Use detectWorldSpaceChange=false, otherwise some of the data we just loaded would be cleared again
         MWBase::Environment::get().getWorld()->changeToCell (cellId, ptr.getRefData().getPosition(), false);

components/esm/loadcell.cpp

@@ -91,6 +91,21 @@ namespace ESM
 
         if (!hasData)
             esm.fail("Missing DATA subrecord");
+
+        mCellId.mPaged = !(mData.mFlags & Interior);
+
+        if (mCellId.mPaged)
+        {
+            mCellId.mWorldspace = "sys::default";
+            mCellId.mIndex.mX = mData.mX;
+            mCellId.mIndex.mY = mData.mY;
+        }
+        else
+        {
+            mCellId.mWorldspace = Misc::StringUtils::lowerCase (mName);
+            mCellId.mIndex.mX = 0;
+            mCellId.mIndex.mY = 0;
+        }
     }
 
     void Cell::loadCell(ESMReader &esm, bool saveContext)
@@ -266,25 +281,8 @@ namespace ESM
         mAmbi.mFogDensity = 0;
     }
 
-    CellId Cell::getCellId() const
+    const CellId& Cell::getCellId() const
     {
-        CellId id;
+        return mCellId;
-
-        id.mPaged = !(mData.mFlags & Interior);
-
-        if (id.mPaged)
-        {
-            id.mWorldspace = "sys::default";
-            id.mIndex.mX = mData.mX;
-            id.mIndex.mY = mData.mY;
-        }
-        else
-        {
-            id.mWorldspace = Misc::StringUtils::lowerCase (mName);
-            id.mIndex.mX = 0;
-            id.mIndex.mY = 0;
-        }
-
-        return id;
     }
 }

components/esm/loadcell.hpp

@@ -8,6 +8,7 @@
 #include "esmcommon.hpp"
 #include "defs.hpp"
 #include "cellref.hpp"
+#include "cellid.hpp"
 
 namespace MWWorld
 {
@@ -18,7 +19,6 @@ namespace ESM
 {
 class ESMReader;
 class ESMWriter;
-struct CellId;
 
 /* Moved cell reference tracking object. This mainly stores the target cell
         of the reference, so we can easily know where it has been moved when another
@@ -97,6 +97,8 @@ struct Cell
 
   std::vector<ESM_Context> mContextList; // File position; multiple positions for multiple plugin support
   DATAstruct mData;
+  CellId mCellId;
+
   AMBIstruct mAmbi;
 
   float mWater; // Water level
@@ -173,7 +175,7 @@ struct Cell
     void blank();
     ///< Set record to default state (does not touch the ID/index).
 
-    CellId getCellId() const;
+    const CellId& getCellId() const;
 };
 }
 #endif