From: Tim Moore <timoore@redhat.com>
Date: Fri, 13 Nov 2009 13:59:29 +0000 (+0100)
Subject: optimize creation and sharing of effects
X-Git-Url: https://git.mxchange.org/?a=commitdiff_plain;h=5991195062dad2bd11962eb52a49127e5a636b10;p=simgear.git

optimize creation and sharing of effects

Implement equality test and hash for for property trees.  In an
Effect, make a hash table of Effects that inherit from it keyed on
their unmerged property tree. Using that, makeEffect() should return a
single Effect for given property tree description. Animations may
change that in the future...
---

diff --git a/simgear/props/props.cxx b/simgear/props/props.cxx
index 6730ef13..c662eb7f 100644
--- a/simgear/props/props.cxx
+++ b/simgear/props/props.cxx
@@ -16,12 +16,15 @@
 
 #include <sstream>
 #include <iomanip>
+#include <iterator>
 #include <stdio.h>
 #include <string.h>
 
 #include <boost/algorithm/string/find_iterator.hpp>
 #include <boost/algorithm/string/predicate.hpp>
 #include <boost/algorithm/string/classification.hpp>
+#include <boost/bind.hpp>
+#include <boost/functional/hash.hpp>
 #include <boost/range.hpp>
 
 #include <simgear/math/SGMath.hxx>
@@ -2379,6 +2382,146 @@ std::istream& readFrom<SGVec4d>(std::istream& stream, SGVec4d& result)
     }
     return stream;
 }
+
+namespace
+{
+bool compareNodeValue(const SGPropertyNode& lhs, const SGPropertyNode& rhs)
+{
+    props::Type ltype = lhs.getType();
+    props::Type rtype = rhs.getType();
+    if (ltype != rtype)
+        return false;
+    switch (ltype) {
+    case props::NONE:
+        return true;
+    case props::ALIAS:
+        return false;           // XXX Should we look in aliases?
+    case props::BOOL:
+        return lhs.getValue<bool>() == rhs.getValue<bool>();
+    case props::INT:
+        return lhs.getValue<int>() == rhs.getValue<int>();
+    case props::LONG:
+        return lhs.getValue<long>() == rhs.getValue<long>();
+    case props::FLOAT:
+        return lhs.getValue<float>() == rhs.getValue<float>();
+    case props::DOUBLE:
+        return lhs.getValue<double>() == rhs.getValue<double>();
+    case props::STRING:
+    case props::UNSPECIFIED:
+        return !strcmp(lhs.getStringValue(), rhs.getStringValue());
+    case props::VEC3D:
+        return lhs.getValue<SGVec3d>() == rhs.getValue<SGVec3d>();
+    case props::VEC4D:
+        return lhs.getValue<SGVec4d>() == rhs.getValue<SGVec4d>();
+    default:
+        return false;
+    }
+}
+}
+}
+
+bool SGPropertyNode::compare(const SGPropertyNode& lhs,
+                             const SGPropertyNode& rhs)
+{
+    if (&lhs == &rhs)
+        return true;
+    int lhsChildren = lhs.nChildren();
+    int rhsChildren = rhs.nChildren();
+    if (lhsChildren != rhsChildren)
+        return false;
+    if (lhsChildren == 0)
+        return compareNodeValue(lhs, rhs);
+    for (size_t i = 0; i < lhs._children.size(); ++i) {
+        const SGPropertyNode* lchild = lhs._children[i];
+        const SGPropertyNode* rchild = rhs._children[i];
+        // I'm guessing that the nodes will usually be in the same
+        // order.
+        if (lchild->getIndex() != rchild->getIndex()
+            || lchild->getNameString() != rchild->getNameString()) {
+            rchild = 0;
+            for (PropertyList::const_iterator itr = rhs._children.begin(),
+                     end = rhs._children.end();
+                 itr != end;
+                ++itr)
+                if (lchild->getIndex() == (*itr)->getIndex()
+                    && lchild->getNameString() == (*itr)->getNameString()) {
+                    rchild = *itr;
+                    break;
+                }
+            if (!rchild)
+                return false;
+        }
+        if (!compare(*lchild, *rchild))
+            return false;
+    }
+    return true;
+}
+
+struct PropertyPlaceLess {
+    typedef bool result_type;
+    bool operator()(SGPropertyNode_ptr lhs, SGPropertyNode_ptr rhs) const
+    {
+        int comp = lhs->getNameString().compare(rhs->getNameString());
+        if (comp == 0)
+            return lhs->getIndex() < rhs->getIndex();
+        else
+            return comp < 0;
+    }
+};
+
+size_t hash_value(const SGPropertyNode& node)
+{
+    using namespace boost;
+
+    if (node.nChildren() == 0) {
+        switch (node.getType()) {
+        case props::NONE:
+            return 0;
+
+        case props::BOOL:
+            return hash_value(node.getValue<bool>());
+        case props::INT:
+            return hash_value(node.getValue<int>());
+        case props::LONG:
+            return hash_value(node.getValue<long>());
+        case props::FLOAT:
+            return hash_value(node.getValue<float>());
+        case props::DOUBLE:
+            return hash_value(node.getValue<double>());
+        case props::STRING:
+        case props::UNSPECIFIED:
+        {
+            const char *val = node.getStringValue();
+            return hash_range(val, val + strlen(val));
+        }
+        case props::VEC3D:
+        {
+            const SGVec3d val = node.getValue<SGVec3d>();
+            return hash_range(&val[0], &val[3]);
+        }
+        case props::VEC4D:
+        {
+            const SGVec4d val = node.getValue<SGVec4d>();
+            return hash_range(&val[0], &val[4]);
+        }
+        case props::ALIAS:      // XXX Should we look in aliases?
+        default:
+            return 0;
+        }
+    } else {
+        size_t seed = 0;
+        PropertyList children(node._children.begin(), node._children.end());
+        sort(children.begin(), children.end(), PropertyPlaceLess());
+        for (PropertyList::const_iterator itr  = children.begin(),
+                 end = children.end();
+             itr != end;
+             ++itr) {
+            hash_combine(seed, (*itr)->_name);
+            hash_combine(seed, (*itr)->_index);
+            hash_combine(seed, hash_value(**itr));
+        }
+        return seed;
+    }
 }
 
 // end of props.cxx
diff --git a/simgear/props/props.hxx b/simgear/props/props.hxx
index e32d9a64..984bb1b5 100644
--- a/simgear/props/props.hxx
+++ b/simgear/props/props.hxx
@@ -1599,6 +1599,16 @@ public:
    */
   void clearValue ();
 
+  /**
+   * Compare two property trees. The property trees are equal if: 1)
+   * They have no children, and have the same type and the values are
+   * equal, or 2) have the same number of children, and the
+   * corresponding children in each tree are equal. "corresponding"
+   * means have the same name and index.
+   *
+   * Attributes, removed children, and aliases aren't considered.
+   */
+  static bool compare (const SGPropertyNode& lhs, const SGPropertyNode& rhs);
 
 protected:
 
@@ -1615,7 +1625,7 @@ protected:
 
 private:
 
-				// Get the raw value
+  // Get the raw value
   bool get_bool () const;
   int get_int () const;
   long get_long () const;
@@ -1623,7 +1633,7 @@ private:
   double get_double () const;
   const char * get_string () const;
 
-				// Set the raw value
+  // Set the raw value
   bool set_bool (bool value);
   bool set_int (int value);
   bool set_long (long value);
@@ -1672,7 +1682,7 @@ private:
   bool _tied;
   int _attr;
 
-				// The right kind of pointer...
+  // The right kind of pointer...
   union {
     SGPropertyNode * alias;
     SGRaw* val;
@@ -1691,8 +1701,8 @@ private:
 
 
   /**
-    * Register/unregister node that links to this node in its path cache.
-    */
+   * Register/unregister node that links to this node in its path cache.
+   */
   void add_linked_node (hash_table * node) { _linkedNodes.push_back(node); }
   bool remove_linked_node (hash_table * node);
 
@@ -1759,6 +1769,8 @@ private:
   template<typename SplitItr>
   friend SGPropertyNode* find_node_aux(SGPropertyNode * current, SplitItr& itr,
                                        bool create, int last_index);
+  // For boost
+  friend size_t hash_value(const SGPropertyNode& node);
 };
 
 // Convenience functions for use in templates
@@ -1947,6 +1959,49 @@ inline SGPropertyNode* makeNode(SGPropertyNode* parent, const StringType& name)
     return parent->getNode(name, true);
 }
 }
+
+// For boost::hash
+size_t hash_value(const SGPropertyNode& node);
+
+// Helper comparison and hash functions for common cases
+
+namespace simgear
+{
+namespace props
+{
+struct Compare
+{
+    bool operator()(const SGPropertyNode* lhs, const SGPropertyNode* rhs) const
+    {
+        return SGPropertyNode::compare(*lhs, *rhs);
+    }
+    bool operator()(SGPropertyNode_ptr lhs, const SGPropertyNode* rhs) const
+    {
+        return SGPropertyNode::compare(*lhs, *rhs);
+    }
+    bool operator()(const SGPropertyNode* lhs, SGPropertyNode_ptr rhs) const
+    {
+        return SGPropertyNode::compare(*lhs, *rhs);
+    }
+    bool operator()(SGPropertyNode_ptr lhs, SGPropertyNode_ptr rhs) const
+    {
+        return SGPropertyNode::compare(*lhs, *rhs);
+    }
+};
+
+struct Hash
+{
+    size_t operator()(const SGPropertyNode* node) const
+    {
+        return hash_value(*node);
+    }
+    size_t operator()(SGPropertyNode_ptr node) const
+    {
+        return hash_value(*node);
+    }
+};
+}
+}
 #endif // __PROPS_HXX
 
 // end of props.hxx
diff --git a/simgear/scene/material/Effect.cxx b/simgear/scene/material/Effect.cxx
index d75a1ac4..1cff2323 100644
--- a/simgear/scene/material/Effect.cxx
+++ b/simgear/scene/material/Effect.cxx
@@ -78,11 +78,13 @@ using namespace osgUtil;
 using namespace effect;
 
 Effect::Effect()
+    : _cache(0), _isRealized(false)
 {
 }
 
 Effect::Effect(const Effect& rhs, const CopyOp& copyop)
-    : root(rhs.root), parametersProp(rhs.parametersProp)
+    : root(rhs.root), parametersProp(rhs.parametersProp), _cache(0),
+      _isRealized(rhs._isRealized)
 {
     typedef vector<ref_ptr<Technique> > TechniqueList;
     for (TechniqueList::const_iterator itr = rhs.techniques.begin(),
@@ -132,6 +134,7 @@ void Effect::releaseGLObjects(osg::State* state) const
 
 Effect::~Effect()
 {
+    delete _cache;
 }
 
 void buildPass(Effect* effect, Technique* tniq, const SGPropertyNode* prop,
@@ -867,11 +870,14 @@ bool makeParametersFromStateSet(SGPropertyNode* effectRoot, const StateSet* ss)
 // passes.
 bool Effect::realizeTechniques(const osgDB::ReaderWriter::Options* options)
 {
+    if (_isRealized)
+        return true;
     PropertyList tniqList = root->getChildren("technique");
     for (PropertyList::iterator itr = tniqList.begin(), e = tniqList.end();
          itr != e;
          ++itr)
         buildTechnique(this, *itr, options);
+    _isRealized = true;
     return true;
 }
 
@@ -895,6 +901,23 @@ void Effect::InitializeCallback::doUpdate(osg::Node* node, osg::NodeVisitor* nv)
     }
 }
 
+bool Effect::Key::EqualTo::operator()(const Effect::Key& lhs,
+                                      const Effect::Key& rhs) const
+{
+    if (lhs.paths.size() != rhs.paths.size()
+        || !equal(lhs.paths.begin(), lhs.paths.end(), rhs.paths.begin()))
+        return false;
+    return props::Compare()(lhs.unmerged, rhs.unmerged);
+}
+
+size_t hash_value(const Effect::Key& key)
+{
+    size_t seed = 0;
+    boost::hash_combine(seed, *key.unmerged);
+    boost::hash_range(seed, key.paths.begin(), key.paths.end());
+    return seed;
+}
+
 bool Effect_writeLocalData(const Object& obj, osgDB::Output& fw)
 {
     const Effect& effect = static_cast<const Effect&>(obj);
diff --git a/simgear/scene/material/Effect.hxx b/simgear/scene/material/Effect.hxx
index c3436c43..1523dc25 100644
--- a/simgear/scene/material/Effect.hxx
+++ b/simgear/scene/material/Effect.hxx
@@ -20,6 +20,8 @@
 #include <vector>
 #include <string>
 
+#include <boost/unordered_map.hpp>
+
 #include <osg/Object>
 #include <osgDB/ReaderWriter>
 
@@ -104,11 +106,43 @@ public:
     {
         void doUpdate(osg::Node* node, osg::NodeVisitor* nv);
     };
-    
 protected:
     std::vector<SGSharedPtr<Updater> > _extraData;
     ~Effect();
+    // Support for a cache of effects that inherit from this one, so
+    // Effect objects with the same parameters and techniques can be
+    // shared.
+    struct Key
+    {
+        Key(SGPropertyNode* unmerged_, const osgDB::FilePathList& paths_)
+            : unmerged(unmerged_), paths(paths_)
+        {
+        }
+        const SGPropertyNode_ptr unmerged;
+        const osgDB::FilePathList paths;
+        struct EqualTo
+        {
+            bool operator()(const Key& lhs, const Key& rhs) const;
+        };
+    };
+    typedef boost::unordered_map<Key, osg::ref_ptr<Effect>, boost::hash<Key>,
+                                 Key::EqualTo> Cache;
+    Cache* getCache()
+    {
+        if (!_cache)
+            _cache = new Cache;
+        return _cache;
+    }
+    Cache* _cache;
+    friend size_t hash_value(const Key& key);
+    friend Effect* makeEffect(SGPropertyNode* prop, bool realizeTechniques,
+                              const osgDB::ReaderWriter::Options* options);
+    bool _isRealized;
 };
+// Automatic support for boost hash function
+size_t hash_value(const Effect::Key&);
+
+
 Effect* makeEffect(const std::string& name,
                    bool realizeTechniques,
                    const osgDB::ReaderWriter::Options* options = 0);
diff --git a/simgear/scene/material/makeEffect.cxx b/simgear/scene/material/makeEffect.cxx
index 59d3b2d9..d04173b5 100644
--- a/simgear/scene/material/makeEffect.cxx
+++ b/simgear/scene/material/makeEffect.cxx
@@ -165,23 +165,32 @@ Effect* makeEffect(SGPropertyNode* prop,
             }
         }
     }
-    Effect* effect = new Effect;
+    Effect* effect = 0;
     // Merge with the parent effect, if any
-    const SGPropertyNode* inheritProp = prop->getChild("inherits-from");
+    SGPropertyNode_ptr inheritProp = prop->getChild("inherits-from");
     Effect* parent = 0;
     if (inheritProp) {
-        parent = makeEffect(inheritProp->getStringValue(), false,
-                            options);
-        if(parent)
-        {
-            effect->root = new SGPropertyNode;
-            mergePropertyTrees(effect->root, prop, parent->root);
-            effect->root->removeChild("inherits-from");
+        //prop->removeChild("inherits-from");
+        parent = makeEffect(inheritProp->getStringValue(), false, options);
+        if (parent) {
+            Effect::Cache* cache = parent->getCache();
+            Effect::Key key(prop, options->getDatabasePathList());
+            Effect::Cache::iterator itr = cache->find(key);
+            if (itr != cache->end()) {
+                effect = itr->second.get();
+            } else {
+                effect = new Effect;
+                effect->root = new SGPropertyNode;
+                mergePropertyTrees(effect->root, prop, parent->root);
+                cache->insert(make_pair(key, effect));
+            }
         } else {
-            effect->root = prop;
-            effect->root->removeChild("inherits-from");
+            SG_LOG(SG_INPUT, SG_WARN, "can't find base effect " <<
+                   inheritProp->getStringValue());
         }
-    } else {
+    }
+    if (!effect) {
+        effect = new Effect;
         effect->root = prop;
     }
     effect->parametersProp = effect->root->getChild("parameters");