From ece38a6dbf4c4d7d4ab042f9096b2d69b02412d1 Mon Sep 17 00:00:00 2001 From: ThorstenB Date: Wed, 28 Mar 2012 22:53:30 +0200 Subject: [PATCH] Fix growing memory consumption issues References in simgear's cache maps prevented effects, textures, clouds, trees and lights from being unloaded at run-time (ref-counter could never reach 0). Changed cache maps to use OSG 'observer' pointers instead, so reference counters aren't influenced, though cache maps still hold an (observing) pointer. Also take care when finding a cache entry with empty content (i.e. texture was unloaded). --- simgear/scene/material/Effect.cxx | 5 +++-- simgear/scene/material/Effect.hxx | 3 ++- simgear/scene/material/TextureBuilder.cxx | 15 ++++++++++++--- simgear/scene/material/makeEffect.cxx | 17 ++++++++++++----- simgear/scene/sky/newcloud.cxx | 22 ++++++++++++++++------ simgear/scene/sky/newcloud.hxx | 3 +-- simgear/scene/tgdb/TreeBin.cxx | 20 +++++++++++++------- simgear/scene/tgdb/pt_lights.cxx | 14 +++++++++----- 8 files changed, 68 insertions(+), 31 deletions(-) diff --git a/simgear/scene/material/Effect.cxx b/simgear/scene/material/Effect.cxx index 16458407..fa99ccb5 100644 --- a/simgear/scene/material/Effect.cxx +++ b/simgear/scene/material/Effect.cxx @@ -1392,10 +1392,11 @@ public: void valueChanged(SGPropertyNode* node) { - _tniq->refreshValidity(); + if (_tniq.valid()) + _tniq->refreshValidity(); } protected: - osg::ref_ptr _tniq; + osg::observer_ptr _tniq; }; template diff --git a/simgear/scene/material/Effect.hxx b/simgear/scene/material/Effect.hxx index 6daa279f..ab795289 100644 --- a/simgear/scene/material/Effect.hxx +++ b/simgear/scene/material/Effect.hxx @@ -24,6 +24,7 @@ #include #include +#include #include #include @@ -158,7 +159,7 @@ protected: bool operator()(const Key& lhs, const Key& rhs) const; }; }; - typedef std::tr1::unordered_map, + typedef std::tr1::unordered_map, boost::hash, Key::EqualTo> Cache; Cache* getCache() { diff --git a/simgear/scene/material/TextureBuilder.cxx b/simgear/scene/material/TextureBuilder.cxx index 8e90064a..88355e36 100644 --- a/simgear/scene/material/TextureBuilder.cxx +++ b/simgear/scene/material/TextureBuilder.cxx @@ -279,7 +279,7 @@ public: Texture* build(Effect* effect, Pass* pass, const SGPropertyNode*, const SGReaderWriterOptions* options); protected: - typedef map > TexMap; + typedef map > TexMap; TexMap texMap; const string _type; }; @@ -290,11 +290,20 @@ Texture* TexBuilder::build(Effect* effect, Pass* pass, const SGPropertyNode* { TexTuple attrs = makeTexTuple(effect, props, options, _type); typename TexMap::iterator itr = texMap.find(attrs); + if (itr != texMap.end()) - return itr->second.get(); + { + T* tex = itr->second.get(); + if (tex) + return tex; + } + T* tex = new T; setAttrs(attrs, tex, options); - texMap.insert(make_pair(attrs, tex)); + if (itr == texMap.end()) + texMap.insert(make_pair(attrs, tex)); + else + itr->second = tex; // update existing, but empty observer return tex; } diff --git a/simgear/scene/material/makeEffect.cxx b/simgear/scene/material/makeEffect.cxx index 477328ff..4ca288b8 100644 --- a/simgear/scene/material/makeEffect.cxx +++ b/simgear/scene/material/makeEffect.cxx @@ -44,7 +44,7 @@ using namespace osg; using namespace effect; typedef vector RawPropVector; -typedef map > EffectMap; +typedef map > EffectMap; namespace { @@ -122,7 +122,8 @@ Effect* makeEffect(const string& name, { OpenThreads::ScopedLock lock(effectMutex); EffectMap::iterator itr = effectMap.find(name); - if (itr != effectMap.end()) + if ((itr != effectMap.end())&& + itr->second.valid()) return itr->second.get(); } string effectFileName(name); @@ -207,7 +208,8 @@ Effect* makeEffect(SGPropertyNode* prop, itr = cache->find(key); if (itr != cache->end()) { effect = itr->second.get(); - effect->generator = parent->generator; // Copy the generators + if (effect.valid()) + effect->generator = parent->generator; // Copy the generators } } if (!effect.valid()) { @@ -219,8 +221,13 @@ Effect* makeEffect(SGPropertyNode* prop, lock(effectMutex); pair irslt = cache->insert(make_pair(key, effect)); - if (!irslt.second) - effect = irslt.first->second; + if (!irslt.second) { + ref_ptr old = irslt.first->second.get(); + if (old.valid()) + effect = old; // Another thread beat us in creating it! Discard our own... + else + irslt.first->second = effect; // update existing, but empty observer + } effect->generator = parent->generator; // Copy the generators } } else { diff --git a/simgear/scene/sky/newcloud.cxx b/simgear/scene/sky/newcloud.cxx index 04fb2207..02f04c19 100644 --- a/simgear/scene/sky/newcloud.cxx +++ b/simgear/scene/sky/newcloud.cxx @@ -64,7 +64,7 @@ using namespace std; namespace { -typedef std::map > EffectMap; +typedef std::map > EffectMap; EffectMap effectMap; } @@ -102,7 +102,13 @@ SGNewCloud::SGNewCloud(const SGPath &texture_root, const SGPropertyNode *cld_def // Create a new Effect for the texture, if required. EffectMap::iterator iter = effectMap.find(texture); - if (iter == effectMap.end()) { + + if (iter != effectMap.end()) { + effect = iter->second.get(); + } + + if (!effect.valid()) + { SGPropertyNode_ptr pcloudEffect = new SGPropertyNode; makeChild(pcloudEffect, "inherits-from")->setValue("Effects/cloud"); setValue(makeChild(makeChild(makeChild(pcloudEffect, "parameters"), @@ -111,10 +117,14 @@ SGNewCloud::SGNewCloud(const SGPath &texture_root, const SGPropertyNode *cld_def texture); ref_ptr options; options = SGReaderWriterOptions::fromPath(texture_root.str()); - if ((effect = makeEffect(pcloudEffect, true, options.get()))) - effectMap.insert(EffectMap::value_type(texture, effect)); - } else { - effect = iter->second.get(); + effect = makeEffect(pcloudEffect, true, options.get()); + if (effect.valid()) + { + if (iter == effectMap.end()) + effectMap.insert(EffectMap::value_type(texture, effect)); + else + iter->second = effect; // update existing, but empty observer + } } } diff --git a/simgear/scene/sky/newcloud.hxx b/simgear/scene/sky/newcloud.hxx index f6e55733..1f207d41 100644 --- a/simgear/scene/sky/newcloud.hxx +++ b/simgear/scene/sky/newcloud.hxx @@ -84,7 +84,7 @@ private: // The density of the cloud is the shading applied // to cloud sprites on the opposite side of the cloud - // from the sun. For an invidual cloud instance a value + // from the sun. For an individual cloud instance a value // between min_density and max_density is chosen. float min_density; float max_density; @@ -102,7 +102,6 @@ private: static float sprite_density; osg::Geometry* createOrthQuad(float w, float h, int varieties_x, int varieties_y); - }; diff --git a/simgear/scene/tgdb/TreeBin.cxx b/simgear/scene/tgdb/TreeBin.cxx index f8980c6d..a0203ebc 100644 --- a/simgear/scene/tgdb/TreeBin.cxx +++ b/simgear/scene/tgdb/TreeBin.cxx @@ -208,7 +208,7 @@ void addTreeToLeafGeode(Geode* geode, const SGVec3f& p) } } -typedef std::map > EffectMap; +typedef std::map > EffectMap; static EffectMap treeEffectMap; @@ -299,9 +299,14 @@ osg::Group* createForest(SGTreeBinList& forestList, const osg::Matrix& transform for (i = forestList.begin(); i != forestList.end(); ++i) { TreeBin* forest = *i; - Effect* effect = 0; + ref_ptr effect; EffectMap::iterator iter = treeEffectMap.find(forest->texture); - if (iter == treeEffectMap.end()) { + + if (iter != treeEffectMap.end()) + effect = iter->second.get(); + + if (!effect.valid()) + { SGPropertyNode_ptr effectProp = new SGPropertyNode; makeChild(effectProp, "inherits-from")->setStringValue("Effects/tree"); SGPropertyNode* params = makeChild(effectProp, "parameters"); @@ -309,11 +314,12 @@ osg::Group* createForest(SGTreeBinList& forestList, const osg::Matrix& transform params->getChild("texture", 0, true)->getChild("image", 0, true) ->setStringValue(forest->texture); effect = makeEffect(effectProp, true, options); - treeEffectMap.insert(EffectMap::value_type(forest->texture, effect)); - } else { - effect = iter->second.get(); + if (iter == treeEffectMap.end()) + treeEffectMap.insert(EffectMap::value_type(forest->texture, effect)); + else + iter->second = effect; // update existing, but empty observer } - + // Now, create a quadtree for the forest. ShaderGeometryQuadtree quadtree(GetTreeCoord(), AddTreesLeafObject(), diff --git a/simgear/scene/tgdb/pt_lights.cxx b/simgear/scene/tgdb/pt_lights.cxx index 59003fcb..e4ec0d59 100644 --- a/simgear/scene/tgdb/pt_lights.cxx +++ b/simgear/scene/tgdb/pt_lights.cxx @@ -159,7 +159,7 @@ gen_standard_light_sprite(void) namespace { typedef boost::tuple PointParams; -typedef std::map > EffectMap; +typedef std::map > EffectMap; EffectMap effectMap; @@ -174,7 +174,8 @@ Effect* getLightEffect(float size, const Vec3& attenuation, PointParams pointParams(size, attenuation, minSize, maxSize, directional); ScopedLock lock(lightMutex); EffectMap::iterator eitr = effectMap.find(pointParams); - if (eitr != effectMap.end()) + if ((eitr != effectMap.end())&& + eitr->second.valid()) return eitr->second.get(); // Basic stuff; no sprite or attenuation support Pass *basicPass = new Pass; @@ -204,7 +205,7 @@ Effect* getLightEffect(float size, const Vec3& attenuation, spritePass->setTextureAttribute(0, attrFact->getStandardTexEnv()); Pass *combinedPass = clone(spritePass, CopyOp::SHALLOW_COPY); combinedPass->setAttributeAndModes(point); - Effect* effect = new Effect; + ref_ptr effect = new Effect; std::vector parameterExtensions; if (SGSceneFeatures::instance()->getEnablePointSpriteLights()) @@ -232,8 +233,11 @@ Effect* getLightEffect(float size, const Vec3& attenuation, Technique* basicTniq = new Technique(true); basicTniq->passes.push_back(basicPass); effect->techniques.push_back(basicTniq); - effectMap.insert(std::make_pair(pointParams, effect)); - return effect; + if (eitr == effectMap.end()) + effectMap.insert(std::make_pair(pointParams, effect)); + else + eitr->second = effect; // update existing, but empty observer + return effect.release(); } -- 2.39.5