From fbee32729acb8af7e397b927031ac09e605ca84a Mon Sep 17 00:00:00 2001
From: scrawl <scrawl@baseoftrash.de>
Date: Thu, 26 Nov 2015 23:46:41 +0100
Subject: [PATCH] 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 |  2 +-
 components/esm/loadcell.cpp             | 36 ++++++++++++-------------
 components/esm/loadcell.hpp             |  6 +++--
 3 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/apps/openmw/mwstate/statemanagerimp.cpp b/apps/openmw/mwstate/statemanagerimp.cpp
index 6a780f2ca..b76d9eae5 100644
--- a/apps/openmw/mwstate/statemanagerimp.cpp
+++ b/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);
diff --git a/components/esm/loadcell.cpp b/components/esm/loadcell.cpp
index 8455daeac..eb9d6b8d8 100644
--- a/components/esm/loadcell.cpp
+++ b/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;
-
-        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;
+        return mCellId;
     }
 }
diff --git a/components/esm/loadcell.hpp b/components/esm/loadcell.hpp
index 2a7a78a54..f92e0b5b7 100644
--- a/components/esm/loadcell.hpp
+++ b/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