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).

diff --git a/simgear/scene/material/Effect.cxx b/simgear/scene/material/Effect.cxx
index 1645840725607d68b9f040becee05c15f6461de2..fa99ccb5b2cadb346bf23ba053a80d347d6491e7 100644 (file)
--- 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<Technique> _tniq;
+    osg::observer_ptr<Technique> _tniq;
 };
 
 template<typename T>

diff --git a/simgear/scene/material/Effect.hxx b/simgear/scene/material/Effect.hxx
index 6daa279f157e0eb86bb56a3ed7f65be1e14a97f8..ab795289d4f32cc6f75eca1e4ef6a95330100ba7 100644 (file)
--- a/simgear/scene/material/Effect.hxx
+++ b/simgear/scene/material/Effect.hxx
@@ -24,6 +24,7 @@
 #include <boost/functional/hash.hpp>
 
 #include <osg/Object>
+#include <osg/observer_ptr>
 #include <osgDB/ReaderWriter>
 
 #include <simgear/props/props.hxx>
@@ -158,7 +159,7 @@ protected:
             bool operator()(const Key& lhs, const Key& rhs) const;
         };
     };
-    typedef std::tr1::unordered_map<Key, osg::ref_ptr<Effect>,
+    typedef std::tr1::unordered_map<Key, osg::observer_ptr<Effect>,
                                     boost::hash<Key>, Key::EqualTo> Cache;
     Cache* getCache()
     {

diff --git a/simgear/scene/material/TextureBuilder.cxx b/simgear/scene/material/TextureBuilder.cxx
index 8e90064ab626436252c97b3787f7c3caf653558b..88355e3685014d772855fbf84dce8d5fe414f8dc 100644 (file)
--- 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<TexTuple, ref_ptr<T> > TexMap;
+    typedef map<TexTuple, observer_ptr<T> > TexMap;
     TexMap texMap;
     const string _type;
 };
@@ -290,11 +290,20 @@ Texture* TexBuilder<T>::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 477328ff495daf649680573c2218cdb249870582..4ca288b87f5603dbcf8a9c11a50f0a53b7719e5b 100644 (file)
--- a/simgear/scene/material/makeEffect.cxx
+++ b/simgear/scene/material/makeEffect.cxx
@@ -44,7 +44,7 @@ using namespace osg;
 using namespace effect;
 
 typedef vector<const SGPropertyNode*> RawPropVector;
-typedef map<const string, ref_ptr<Effect> > EffectMap;
+typedef map<const string, observer_ptr<Effect> > EffectMap;
 
 namespace
 {
@@ -122,7 +122,8 @@ Effect* makeEffect(const string& name,
     {
         OpenThreads::ScopedLock<OpenThreads::ReentrantMutex> 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<Effect::Cache::iterator, bool> irslt
                     = cache->insert(make_pair(key, effect));
-                if (!irslt.second)
-                    effect = irslt.first->second;
+                if (!irslt.second) {
+                    ref_ptr<Effect> 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 04fb2207c8409ab4092410816484410a9d3c5f9f..02f04c197f10a41d8d7ce75f64c7a938286c8c79 100644 (file)
--- a/simgear/scene/sky/newcloud.cxx
+++ b/simgear/scene/sky/newcloud.cxx
@@ -64,7 +64,7 @@ using namespace std;
 
 namespace
 {
-typedef std::map<std::string, osg::ref_ptr<Effect> > EffectMap;
+typedef std::map<std::string, osg::observer_ptr<Effect> > 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<SGReaderWriterOptions> 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 f6e55733dec0bff9479867eb2182bb657c108523..1f207d412a7d89c66fa165444c8c1a2ae76af3fd 100644 (file)
--- 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 f8980c6d552f9586169ae3cfdcf130969640c576..a0203ebce561f91d25696e3cf55302a32ce25b2b 100644 (file)
--- 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<std::string, osg::ref_ptr<Effect> > EffectMap;
+typedef std::map<std::string, osg::observer_ptr<Effect> > 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> 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 59003fcb36279df4195b39df69c464d76c3959a8..e4ec0d5995ae7de00936ffd9a3f603e121666ca2 100644 (file)
--- 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<float, osg::Vec3, float, float, bool> PointParams;
-typedef std::map<PointParams, ref_ptr<Effect> > EffectMap;
+typedef std::map<PointParams, observer_ptr<Effect> > EffectMap;
 
 EffectMap effectMap;
 
@@ -174,7 +174,8 @@ Effect* getLightEffect(float size, const Vec3& attenuation,
     PointParams pointParams(size, attenuation, minSize, maxSize, directional);
     ScopedLock<Mutex> 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> effect = new Effect;
     std::vector<std::string> 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();
 }