removes UnrefQueue (#3181)

Currently, we use an `UnrefQueue` which supposedly aims to transfer destruction costs to another thread. The implications of this unusual pattern can not be well understood because some allocators might free resources more efficiently if they are freed by the same thread that allocated them. In addition, `UnrefQueue` complicates the validation of thread safety in our engine. Lastly, our current usage of `UnrefQueue` triggers `ref()`, `unref()` atomic operations as objects are passed into the queue. These operations could be more expensive than the actual destruction.

With this PR we thus remove `UnrefQueue`. We can expect these changes to have a minor impact at most because we free most resources elsewhere in `ResourceSystem::updateCache`.
pull/3185/head
Bo Svensson 3 years ago committed by GitHub
parent fc32793cc6
commit a854a6e04a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -24,7 +24,6 @@
#include <components/debug/debuglog.hpp>
#include <components/esm/loadgmst.hpp>
#include <components/sceneutil/positionattitudetransform.hpp>
#include <components/sceneutil/unrefqueue.hpp>
#include <components/misc/convert.hpp>
#include <components/nifosg/particle.hpp> // FindRecIndexVisitor
@ -160,11 +159,6 @@ namespace MWPhysics
mProjectiles.clear();
}
void PhysicsSystem::setUnrefQueue(SceneUtil::UnrefQueue *unrefQueue)
{
mUnrefQueue = unrefQueue;
}
Resource::BulletShapeManager *PhysicsSystem::getShapeManager()
{
return mShapeManager.get();
@ -513,9 +507,6 @@ namespace MWPhysics
{
if (auto foundObject = mObjects.find(ptr.mRef); foundObject != mObjects.end())
{
if (mUnrefQueue.get())
mUnrefQueue->push(foundObject->second->getShapeInstance());
mAnimatedObjects.erase(foundObject->second.get());
mObjects.erase(foundObject);

@ -36,11 +36,6 @@ namespace Resource
class ResourceSystem;
}
namespace SceneUtil
{
class UnrefQueue;
}
class btCollisionWorld;
class btBroadphaseInterface;
class btDefaultCollisionConfiguration;
@ -118,8 +113,6 @@ namespace MWPhysics
PhysicsSystem (Resource::ResourceSystem* resourceSystem, osg::ref_ptr<osg::Group> parentNode);
virtual ~PhysicsSystem ();
void setUnrefQueue(SceneUtil::UnrefQueue* unrefQueue);
Resource::BulletShapeManager* getShapeManager();
void enableWater(float height);
@ -262,8 +255,6 @@ namespace MWPhysics
std::pair<std::vector<std::shared_ptr<Actor>>, std::vector<ActorFrameData>> prepareFrameData(bool willSimulate);
osg::ref_ptr<SceneUtil::UnrefQueue> mUnrefQueue;
std::unique_ptr<btBroadphaseInterface> mBroadphase;
std::unique_ptr<btDefaultCollisionConfiguration> mCollisionConfiguration;
std::unique_ptr<btCollisionDispatcher> mDispatcher;

@ -4,7 +4,6 @@
#include <osg/UserDataContainer>
#include <components/sceneutil/positionattitudetransform.hpp>
#include <components/sceneutil/unrefqueue.hpp>
#include "../mwworld/ptr.hpp"
#include "../mwworld/class.hpp"
@ -18,10 +17,9 @@
namespace MWRender
{
Objects::Objects(Resource::ResourceSystem* resourceSystem, osg::ref_ptr<osg::Group> rootNode, SceneUtil::UnrefQueue* unrefQueue)
Objects::Objects(Resource::ResourceSystem* resourceSystem, osg::ref_ptr<osg::Group> rootNode)
: mRootNode(rootNode)
, mResourceSystem(resourceSystem)
, mUnrefQueue(unrefQueue)
{
}
@ -117,9 +115,6 @@ bool Objects::removeObject (const MWWorld::Ptr& ptr)
PtrAnimationMap::iterator iter = mObjects.find(ptr);
if(iter != mObjects.end())
{
if (mUnrefQueue.get())
mUnrefQueue->push(iter->second);
mObjects.erase(iter);
if (ptr.getClass().isActor())
@ -146,9 +141,6 @@ void Objects::removeCell(const MWWorld::CellStore* store)
MWWorld::Ptr ptr = iter->second->getPtr();
if(ptr.getCell() == store)
{
if (mUnrefQueue.get())
mUnrefQueue->push(iter->second);
if (ptr.getClass().isNpc() && ptr.getRefData().getCustomData())
{
MWWorld::InventoryStore& invStore = ptr.getClass().getInventoryStore(ptr);
@ -166,8 +158,6 @@ void Objects::removeCell(const MWWorld::CellStore* store)
if(cell != mCellSceneNodes.end())
{
cell->second->getParent(0)->removeChild(cell->second);
if (mUnrefQueue.get())
mUnrefQueue->push(cell->second);
mCellSceneNodes.erase(cell);
}
}

@ -24,11 +24,6 @@ namespace MWWorld
class CellStore;
}
namespace SceneUtil
{
class UnrefQueue;
}
namespace MWRender{
class Animation;
@ -66,12 +61,10 @@ class Objects{
Resource::ResourceSystem* mResourceSystem;
osg::ref_ptr<SceneUtil::UnrefQueue> mUnrefQueue;
void insertBegin(const MWWorld::Ptr& ptr);
public:
Objects(Resource::ResourceSystem* resourceSystem, osg::ref_ptr<osg::Group> rootNode, SceneUtil::UnrefQueue* unrefQueue);
Objects(Resource::ResourceSystem* resourceSystem, osg::ref_ptr<osg::Group> rootNode);
~Objects();
/// @param animated Attempt to load separate keyframes from a .kf file matching the model file?

@ -37,7 +37,6 @@
#include <components/sceneutil/statesetupdater.hpp>
#include <components/sceneutil/positionattitudetransform.hpp>
#include <components/sceneutil/workqueue.hpp>
#include <components/sceneutil/unrefqueue.hpp>
#include <components/sceneutil/writescene.hpp>
#include <components/sceneutil/shadow.hpp>
@ -289,7 +288,6 @@ namespace MWRender
, mRootNode(rootNode)
, mResourceSystem(resourceSystem)
, mWorkQueue(workQueue)
, mUnrefQueue(new SceneUtil::UnrefQueue)
, mNavigator(navigator)
, mMinimumAmbientLuminance(0.f)
, mNightEyeFactor(0.f)
@ -387,7 +385,7 @@ namespace MWRender
mRecastMesh.reset(new RecastMesh(mRootNode, Settings::Manager::getBool("enable recast mesh render", "Navigator")));
mPathgrid.reset(new Pathgrid(mRootNode));
mObjects.reset(new Objects(mResourceSystem, sceneRoot, mUnrefQueue.get()));
mObjects.reset(new Objects(mResourceSystem, sceneRoot));
if (getenv("OPENMW_DONT_PRECOMPILE") == nullptr)
{
@ -434,7 +432,6 @@ namespace MWRender
mTerrain.reset(new Terrain::TerrainGrid(sceneRoot, mRootNode, mResourceSystem, mTerrainStorage.get(), Mask_Terrain, Mask_PreCompile, Mask_Debug));
mTerrain->setTargetFrameRate(Settings::Manager::getFloat("target framerate", "Cells"));
mTerrain->setWorkQueue(mWorkQueue.get());
if (groundcover)
{
@ -569,11 +566,6 @@ namespace MWRender
return mWorkQueue.get();
}
SceneUtil::UnrefQueue* RenderingManager::getUnrefQueue()
{
return mUnrefQueue.get();
}
Terrain::World* RenderingManager::getTerrain()
{
return mTerrain.get();
@ -804,8 +796,6 @@ namespace MWRender
{
reportStats();
mUnrefQueue->flush(mWorkQueue.get());
float rainIntensity = mSky->getPrecipitationAlpha();
mWater->setRainIntensity(rainIntensity);
@ -1223,8 +1213,6 @@ namespace MWRender
unsigned int frameNumber = mViewer->getFrameStamp()->getFrameNumber();
if (stats->collectStats("resource"))
{
stats->setAttribute(frameNumber, "UnrefQueue", mUnrefQueue->getNumItems());
mTerrain->reportStats(frameNumber, stats);
}
}

@ -59,7 +59,6 @@ namespace SceneUtil
{
class ShadowManager;
class WorkQueue;
class UnrefQueue;
}
namespace DetourNavigator
@ -106,7 +105,6 @@ namespace MWRender
Resource::ResourceSystem* getResourceSystem();
SceneUtil::WorkQueue* getWorkQueue();
SceneUtil::UnrefQueue* getUnrefQueue();
Terrain::World* getTerrain();
void preloadCommonAssets();
@ -262,7 +260,6 @@ namespace MWRender
Resource::ResourceSystem* mResourceSystem;
osg::ref_ptr<SceneUtil::WorkQueue> mWorkQueue;
osg::ref_ptr<SceneUtil::UnrefQueue> mUnrefQueue;
osg::ref_ptr<osg::Light> mSunLight;

@ -12,7 +12,6 @@
#include <components/misc/resourcehelpers.hpp>
#include <components/misc/stringops.hpp>
#include <components/terrain/world.hpp>
#include <components/sceneutil/unrefqueue.hpp>
#include <components/esm/loadcell.hpp>
#include <components/loadinglistener/reporter.hpp>
@ -322,11 +321,10 @@ namespace MWWorld
PreloadMap::iterator found = mPreloadCells.find(cell);
if (found != mPreloadCells.end())
{
// do the deletion in the background thread
if (found->second.mWorkItem)
{
found->second.mWorkItem->abort();
mUnrefQueue->push(std::move(found->second.mWorkItem));
found->second.mWorkItem = nullptr;
}
mPreloadCells.erase(found);
@ -340,7 +338,7 @@ namespace MWWorld
if (it->second.mWorkItem)
{
it->second.mWorkItem->abort();
mUnrefQueue->push(it->second.mWorkItem);
it->second.mWorkItem = nullptr;
}
mPreloadCells.erase(it++);
@ -356,7 +354,7 @@ namespace MWWorld
if (it->second.mWorkItem)
{
it->second.mWorkItem->abort();
mUnrefQueue->push(it->second.mWorkItem);
it->second.mWorkItem = nullptr;
}
mPreloadCells.erase(it++);
}
@ -409,11 +407,6 @@ namespace MWWorld
mWorkQueue = workQueue;
}
void CellPreloader::setUnrefQueue(SceneUtil::UnrefQueue* unrefQueue)
{
mUnrefQueue = unrefQueue;
}
bool CellPreloader::syncTerrainLoad(const std::vector<CellPreloader::PositionCellGrid> &positions, double timestamp, Loading::Listener& listener)
{
if (!mTerrainPreloadItem)
@ -455,11 +448,7 @@ namespace MWWorld
else
{
if (mTerrainViews.size() > positions.size())
{
for (unsigned int i=positions.size(); i<mTerrainViews.size(); ++i)
mUnrefQueue->push(mTerrainViews[i]);
mTerrainViews.resize(positions.size());
}
else if (mTerrainViews.size() < positions.size())
{
for (unsigned int i=mTerrainViews.size(); i<positions.size(); ++i)

@ -19,11 +19,6 @@ namespace Terrain
class View;
}
namespace SceneUtil
{
class UnrefQueue;
}
namespace MWRender
{
class LandManager;
@ -72,8 +67,6 @@ namespace MWWorld
void setWorkQueue(osg::ref_ptr<SceneUtil::WorkQueue> workQueue);
void setUnrefQueue(SceneUtil::UnrefQueue* unrefQueue);
typedef std::pair<osg::Vec3f, osg::Vec4i> PositionCellGrid;
void setTerrainPreloadPositions(const std::vector<PositionCellGrid>& positions);
@ -87,7 +80,6 @@ namespace MWWorld
Terrain::World* mTerrain;
MWRender::LandManager* mLandManager;
osg::ref_ptr<SceneUtil::WorkQueue> mWorkQueue;
osg::ref_ptr<SceneUtil::UnrefQueue> mUnrefQueue;
double mExpiryDelay;
unsigned int mMinCacheSize;
unsigned int mMaxCacheSize;

@ -15,7 +15,6 @@
#include <components/resource/resourcesystem.hpp>
#include <components/resource/scenemanager.hpp>
#include <components/resource/bulletshape.hpp>
#include <components/sceneutil/unrefqueue.hpp>
#include <components/sceneutil/positionattitudetransform.hpp>
#include <components/detournavigator/navigator.hpp>
#include <components/detournavigator/debug.hpp>
@ -654,7 +653,6 @@ namespace MWWorld
}
mRendering.getResourceSystem()->updateCache(mRendering.getReferenceTime());
mRendering.getUnrefQueue()->flush(mRendering.getWorkQueue());
loadingListener->increaseProgress (1);
i++;
@ -701,7 +699,6 @@ namespace MWWorld
}
mRendering.getResourceSystem()->updateCache(mRendering.getReferenceTime());
mRendering.getUnrefQueue()->flush(mRendering.getWorkQueue());
loadingListener->increaseProgress (1);
i++;
@ -755,9 +752,6 @@ namespace MWWorld
mPreloader.reset(new CellPreloader(rendering.getResourceSystem(), physics->getShapeManager(), rendering.getTerrain(), rendering.getLandManager()));
mPreloader->setWorkQueue(mRendering.getWorkQueue());
mPreloader->setUnrefQueue(rendering.getUnrefQueue());
mPhysics->setUnrefQueue(rendering.getUnrefQueue());
rendering.getResourceSystem()->setExpiryDelay(Settings::Manager::getFloat("cache expiry delay", "Cells"));
mPreloader->setExpiryDelay(Settings::Manager::getFloat("preload cell expiry delay", "Cells"));

@ -1,39 +0,0 @@
#include "unrefqueue.hpp"
//#include <osg/Timer>
//#include <components/debug/debuglog.hpp>
namespace SceneUtil
{
void UnrefWorkItem::doWork()
{
mObjects.clear();
}
UnrefQueue::UnrefQueue()
{
mWorkItem = new UnrefWorkItem;
}
void UnrefQueue::push(const osg::Referenced *obj)
{
mWorkItem->mObjects.emplace_back(obj);
}
void UnrefQueue::flush(SceneUtil::WorkQueue *workQueue)
{
if (mWorkItem->mObjects.empty())
return;
workQueue->addWorkItem(mWorkItem, true);
mWorkItem = new UnrefWorkItem;
}
unsigned int UnrefQueue::getNumItems() const
{
return mWorkItem->mObjects.size();
}
}

@ -1,44 +0,0 @@
#ifndef OPENMW_COMPONENTS_UNREFQUEUE_H
#define OPENMW_COMPONENTS_UNREFQUEUE_H
#include <deque>
#include <osg/ref_ptr>
#include <osg/Referenced>
#include <components/sceneutil/workqueue.hpp>
namespace SceneUtil
{
class WorkQueue;
class UnrefWorkItem : public SceneUtil::WorkItem
{
public:
std::deque<osg::ref_ptr<const osg::Referenced> > mObjects;
void doWork() override;
};
/// @brief Handles unreferencing of objects through the WorkQueue. Typical use scenario
/// would be the main thread pushing objects that are no longer needed, and the background thread deleting them.
class UnrefQueue : public osg::Referenced
{
public:
UnrefQueue();
/// Adds an object to the list of objects to be unreferenced. Call from the main thread.
void push(const osg::Referenced* obj);
/// Adds a WorkItem to the given WorkQueue that will clear the list of objects in a worker thread, thus unreferencing them.
/// Call from the main thread.
void flush(SceneUtil::WorkQueue* workQueue);
unsigned int getNumItems() const;
private:
osg::ref_ptr<UnrefWorkItem> mWorkItem;
};
}
#endif

@ -4,9 +4,6 @@
#include <osg/Texture2D>
#include <osg/RenderInfo>
#include <components/sceneutil/unrefqueue.hpp>
#include <components/sceneutil/workqueue.hpp>
#include <algorithm>
namespace Terrain
@ -21,8 +18,6 @@ CompositeMapRenderer::CompositeMapRenderer()
mFBO = new osg::FrameBufferObject;
mUnrefQueue = new SceneUtil::UnrefQueue;
getOrCreateStateSet()->setMode(GL_LIGHTING, osg::StateAttribute::OFF);
}
@ -30,11 +25,6 @@ CompositeMapRenderer::~CompositeMapRenderer()
{
}
void CompositeMapRenderer::setWorkQueue(SceneUtil::WorkQueue* workQueue)
{
mWorkQueue = workQueue;
}
void CompositeMapRenderer::drawImplementation(osg::RenderInfo &renderInfo) const
{
double dt = mTimer.time_s();
@ -45,9 +35,6 @@ void CompositeMapRenderer::drawImplementation(osg::RenderInfo &renderInfo) const
double availableTime = std::max((targetFrameTime - dt)*conservativeTimeRatio,
mMinimumTimeAvailable);
if (mWorkQueue)
mUnrefQueue->flush(mWorkQueue.get());
std::lock_guard<std::mutex> lock(mMutex);
if (mImmediateCompileSet.empty() && mCompileSet.empty())
@ -139,10 +126,6 @@ void CompositeMapRenderer::compile(CompositeMap &compositeMap, osg::RenderInfo &
++compositeMap.mCompiled;
if (mWorkQueue)
{
mUnrefQueue->push(compositeMap.mDrawables[i]);
}
compositeMap.mDrawables[i] = nullptr;
if (timeLeft)

@ -13,12 +13,6 @@ namespace osg
class Texture2D;
}
namespace SceneUtil
{
class UnrefQueue;
class WorkQueue;
}
namespace Terrain
{
@ -45,9 +39,6 @@ namespace Terrain
void compile(CompositeMap& compositeMap, osg::RenderInfo& renderInfo, double* timeLeft) const;
/// Set a WorkQueue to delete compiled composite map layers in the background thread
void setWorkQueue(SceneUtil::WorkQueue* workQueue);
/// Set the available time in seconds for compiling (non-immediate) composite maps each frame
void setMinimumTimeAvailableForCompile(double time);
@ -67,9 +58,6 @@ namespace Terrain
double mMinimumTimeAvailable;
mutable osg::Timer mTimer;
osg::ref_ptr<SceneUtil::UnrefQueue> mUnrefQueue;
osg::ref_ptr<SceneUtil::WorkQueue> mWorkQueue;
typedef std::set<osg::ref_ptr<CompositeMap> > CompileSet;
mutable CompileSet mCompileSet;

@ -84,11 +84,6 @@ World::~World()
}
}
void World::setWorkQueue(SceneUtil::WorkQueue* workQueue)
{
mCompositeMapRenderer->setWorkQueue(workQueue);
}
void World::setBordersVisible(bool visible)
{
mBorderVisible = visible;

@ -28,11 +28,6 @@ namespace Resource
class ResourceSystem;
}
namespace SceneUtil
{
class WorkQueue;
}
namespace Loading
{
class Reporter;
@ -115,9 +110,6 @@ namespace Terrain
World(osg::Group* parent, Storage* storage, unsigned int nodeMask);
virtual ~World();
/// Set a WorkQueue to delete objects in the background thread.
void setWorkQueue(SceneUtil::WorkQueue* workQueue);
/// See CompositeMapRenderer::setTargetFrameRate
void setTargetFrameRate(float rate);

Loading…
Cancel
Save