separate out CacheKey and CacheEntry

panda/src/gobj/geom.I

@@ -373,47 +373,24 @@ set_result(const Geom *geom_result, const GeomVertexData *data_result) {
 }
 
 ////////////////////////////////////////////////////////////////////
-//     Function: Geom::CacheEntry::Constructor
-//       Access: Public
-//  Description: This constructor makes an invalid CacheEntry that
-//               holds no data.  CacheEntries like this are normally
-//               not stored in the cache; this is usually used as a
-//               key to find an existing (valid) CacheEntry already in
-//               the cache.
-//
-//               However, it is possible for an empty CacheEntry to
-//               end up in the cache if its data gets cleared by
-//               clear_cache_stage().
-////////////////////////////////////////////////////////////////////
-INLINE Geom::CacheEntry::
-CacheEntry(const GeomVertexData *source_data, const GeomMunger *modifier) :
-  _source(NULL),
-  _source_data(source_data),
-  _modifier(modifier)
-{
-}
-
-////////////////////////////////////////////////////////////////////
-//     Function: Geom::CacheEntry::Constructor
+//     Function: Geom::CacheKey::Constructor
 //       Access: Public
 //  Description: 
 ////////////////////////////////////////////////////////////////////
-INLINE Geom::CacheEntry::
-CacheEntry(Geom *source, const GeomVertexData *source_data,
-           const GeomMunger *modifier) :
-  _source(source),
+INLINE Geom::CacheKey::
+CacheKey(const GeomVertexData *source_data, const GeomMunger *modifier) :
   _source_data(source_data),
   _modifier(modifier)
 {
 }
 
 ////////////////////////////////////////////////////////////////////
-//     Function: Geom::CacheEntry::operator <
+//     Function: Geom::CacheKey::operator <
 //       Access: Public
-//  Description: Provides a unique ordering within the set.
+//  Description: Provides a unique ordering within the map.
 ////////////////////////////////////////////////////////////////////
-INLINE bool Geom::CacheEntry::
-operator < (const CacheEntry &other) const {
+INLINE bool Geom::CacheKey::
+operator < (const CacheKey &other) const {
   if (_modifier != other._modifier) {
     int compare = _modifier->geom_compare_to(*other._modifier);
     if (compare != 0) {
@@ -426,6 +403,19 @@ operator < (const CacheEntry &other) const {
   return 0;
 }
 
+////////////////////////////////////////////////////////////////////
+//     Function: Geom::CacheEntry::Constructor
+//       Access: Public
+//  Description: 
+////////////////////////////////////////////////////////////////////
+INLINE Geom::CacheEntry::
+CacheEntry(Geom *source, const GeomVertexData *source_data,
+           const GeomMunger *modifier) :
+  _source(source),
+  _key(source_data, modifier)
+{
+}
+
 
 ////////////////////////////////////////////////////////////////////
 //     Function: Geom::CData::Constructor

panda/src/gobj/geom.cxx

@@ -806,7 +806,7 @@ clear_cache() {
   for (Cache::iterator ci = _cache.begin();
        ci != _cache.end();
        ++ci) {
-    CacheEntry *entry = (*ci);
+    CacheEntry *entry = (*ci).second;
     entry->erase();
   }
   _cache.clear();
@@ -829,7 +829,7 @@ clear_cache_stage(Thread *current_thread) {
   for (Cache::iterator ci = _cache.begin();
        ci != _cache.end();
        ++ci) {
-    CacheEntry *entry = (*ci);
+    CacheEntry *entry = (*ci).second;
     CDCacheWriter cdata(entry->_cycler, current_thread);
     cdata->set_result(NULL, NULL);
   }
@@ -1244,9 +1244,9 @@ make_copy() const {
 void Geom::CacheEntry::
 evict_callback() {
   MutexHolder holder(_source->_cache_lock);
-  Cache::iterator ci = _source->_cache.find(this);
+  Cache::iterator ci = _source->_cache.find(&_key);
   nassertv(ci != _source->_cache.end());
-  nassertv((*ci) == this);
+  nassertv((*ci).second == this);
   _source->_cache.erase(ci);
 }
 
@@ -1258,7 +1258,7 @@ evict_callback() {
 void Geom::CacheEntry::
 output(ostream &out) const {
   out << "geom " << (void *)_source << ", " 
-      << (const void *)_modifier;
+      << (const void *)_key._modifier;
 }
 
 

panda/src/gobj/geom.h

@@ -37,6 +37,7 @@
 #include "pointerTo.h"
 #include "indirectLess.h"
 #include "pset.h"
+#include "pmap.h"
 #include "boundingVolume.h"
 #include "pStatCollector.h"
 #include "deletedChain.h"
@@ -197,27 +198,38 @@ private:
   typedef CycleDataReader<CDataCache> CDCacheReader;
   typedef CycleDataWriter<CDataCache> CDCacheWriter;
 
-public:  // It is not clear why MSVC7 needs this class to be public.  
+public:
+  // The CacheKey class separates out just the part of CacheEntry that
+  // is used to key the cache entry within the map.  We have this as a
+  // separate class so we can easily look up a new entry in the map,
+  // without having to execute the relatively expensive CacheEntry
+  // constructor.
+  class CacheKey {
+  public:
+    INLINE CacheKey(const GeomVertexData *source_data,
+                    const GeomMunger *modifier);
+    INLINE bool operator < (const CacheKey &other) const;
+
+    CPT(GeomVertexData) _source_data;
+    CPT(GeomMunger) _modifier;
+  };
+  // It is not clear why MSVC7 needs this class to be public.
   class CacheEntry : public GeomCacheEntry {
   public:
-    INLINE CacheEntry(const GeomVertexData *source_data,
-                      const GeomMunger *modifier);
     INLINE CacheEntry(Geom *source, 
                       const GeomVertexData *source_data,
                       const GeomMunger *modifier);
-    INLINE bool operator < (const CacheEntry &other) const;
     ALLOC_DELETED_CHAIN(CacheEntry);
 
     virtual void evict_callback();
     virtual void output(ostream &out) const;
 
     Geom *_source;  // A back pointer to the containing Geom
-    CPT(GeomVertexData) _source_data;
-    CPT(GeomMunger) _modifier;
+    CacheKey _key;
 
     PipelineCycler<CDataCache> _cycler;
   };
-  typedef pset<PT(CacheEntry), IndirectLess<CacheEntry> > Cache;
+  typedef pmap<const CacheKey *, PT(CacheEntry), IndirectLess<CacheKey> > Cache;
 
 private:
   // This is the data that must be cycled between pipeline stages.

panda/src/gobj/geomMunger.cxx

@@ -115,15 +115,14 @@ munge_geom(CPT(Geom) &geom, CPT(GeomVertexData) &data,
   // applied it.
   PT(Geom::CacheEntry) entry;
 
-  Geom::CacheEntry temp_entry(source_data, this);
-  temp_entry.local_object();
+  Geom::CacheKey key(source_data, this);
 
   geom->_cache_lock.lock();
-  Geom::Cache::const_iterator ci = geom->_cache.find(&temp_entry);
+  Geom::Cache::const_iterator ci = geom->_cache.find(&key);
   if (ci == geom->_cache.end()) {
     geom->_cache_lock.release();
   } else {
-    entry = (*ci);
+    entry = (*ci).second;
     geom->_cache_lock.release();
     nassertv(entry->_source == geom);
     
@@ -164,7 +163,7 @@ munge_geom(CPT(Geom) &geom, CPT(GeomVertexData) &data,
     entry = new Geom::CacheEntry(orig_geom, source_data, this);
     {
       MutexHolder holder(orig_geom->_cache_lock);
-      bool inserted = orig_geom->_cache.insert(entry).second;
+      bool inserted = orig_geom->_cache.insert(Geom::Cache::value_type(&entry->_key, entry)).second;
       if (!inserted) {
         // Some other thread must have beat us to the punch.  Never
         // mind.

panda/src/gobj/geomVertexData.I

@@ -403,46 +403,36 @@ CDataCache(const GeomVertexData::CDataCache &copy) :
 }
 
 ////////////////////////////////////////////////////////////////////
-//     Function: GeomVertexData::CacheEntry::Constructor
+//     Function: GeomVertexData::CacheKey::Constructor
 //       Access: Public
-//  Description: This constructor makes an invalid CacheEntry that
-//               holds no data.  CacheEntries like this are normally
-//               not stored in the cache; this is usually used as a
-//               key to find an existing (valid) CacheEntry already in
-//               the cache.
-//
-//               However, it is possible for an empty CacheEntry to
-//               end up in the cache if its data gets cleared by
-//               clear_cache_stage().
+//  Description: 
 ////////////////////////////////////////////////////////////////////
-INLINE GeomVertexData::CacheEntry::
-CacheEntry(const GeomVertexFormat *modifier) :
-  _source(NULL),
+INLINE GeomVertexData::CacheKey::
+CacheKey(const GeomVertexFormat *modifier) :
   _modifier(modifier)
 {
 }
 
 ////////////////////////////////////////////////////////////////////
-//     Function: GeomVertexData::CacheEntry::Constructor
+//     Function: GeomVertexData::CacheKey::operator <
 //       Access: Public
-//  Description: 
+//  Description: Provides a unique ordering within the set.
 ////////////////////////////////////////////////////////////////////
-INLINE GeomVertexData::CacheEntry::
-CacheEntry(GeomVertexData *source,
-           const GeomVertexFormat *modifier) :
-  _source(source),
-  _modifier(modifier)
-{
+INLINE bool GeomVertexData::CacheKey::
+operator < (const CacheKey &other) const {
+  return _modifier < other._modifier;
 }
 
 ////////////////////////////////////////////////////////////////////
-//     Function: GeomVertexData::CacheEntry::operator <
+//     Function: GeomVertexData::CacheEntry::Constructor
 //       Access: Public
-//  Description: Provides a unique ordering within the set.
+//  Description: 
 ////////////////////////////////////////////////////////////////////
-INLINE bool GeomVertexData::CacheEntry::
-operator < (const CacheEntry &other) const {
-  return _modifier < other._modifier;
+INLINE GeomVertexData::CacheEntry::
+CacheEntry(GeomVertexData *source, const GeomVertexFormat *modifier) :
+  _source(source),
+  _key(modifier)
+{
 }
 
 ////////////////////////////////////////////////////////////////////

panda/src/gobj/geomVertexData.cxx

@@ -675,16 +675,15 @@ convert_to(const GeomVertexFormat *new_format) const {
   // it.
   PT(CacheEntry) entry;
 
-  CacheEntry temp_entry(new_format);
-  temp_entry.local_object();
+  CacheKey key(new_format);
 
   _cache_lock.lock();
-  Cache::const_iterator ci = _cache.find(&temp_entry);
+  Cache::const_iterator ci = _cache.find(&key);
   if (ci == _cache.end()) {
     _cache_lock.release();
 
   } else {
-    entry = (*ci);
+    entry = (*ci).second;
     _cache_lock.release();
     nassertr(entry->_source == this, NULL);
 
@@ -725,7 +724,7 @@ convert_to(const GeomVertexFormat *new_format) const {
     entry = new CacheEntry((GeomVertexData *)this, new_format);
     {
       MutexHolder holder(_cache_lock);
-      bool inserted = ((GeomVertexData *)this)->_cache.insert(entry).second;
+      bool inserted = ((GeomVertexData *)this)->_cache.insert(Cache::value_type(&entry->_key, entry)).second;
       if (!inserted) {
         // Some other thread must have beat us to the punch.  Never
         // mind.
@@ -1112,7 +1111,7 @@ clear_cache() {
   for (Cache::iterator ci = _cache.begin();
        ci != _cache.end();
        ++ci) {
-    CacheEntry *entry = (*ci);
+    CacheEntry *entry = (*ci).second;
     entry->erase();
   }
   _cache.clear();
@@ -1135,7 +1134,7 @@ clear_cache_stage() {
   for (Cache::iterator ci = _cache.begin();
        ci != _cache.end();
        ++ci) {
-    CacheEntry *entry = (*ci);
+    CacheEntry *entry = (*ci).second;
     CDCacheWriter cdata(entry->_cycler);
     cdata->_result = NULL;
   }
@@ -1487,9 +1486,9 @@ make_copy() const {
 void GeomVertexData::CacheEntry::
 evict_callback() {
   MutexHolder holder(_source->_cache_lock);
-  Cache::iterator ci = _source->_cache.find(this);
+  Cache::iterator ci = _source->_cache.find(&_key);
   nassertv(ci != _source->_cache.end());
-  nassertv((*ci) == this);
+  nassertv((*ci).second == this);
   _source->_cache.erase(ci);
 }
 
@@ -1501,7 +1500,7 @@ evict_callback() {
 void GeomVertexData::CacheEntry::
 output(ostream &out) const {
   out << "vertex data " << (void *)_source << " to " 
-      << *_modifier;
+      << *_key._modifier;
 }
 
 ////////////////////////////////////////////////////////////////////

panda/src/gobj/geomVertexData.h

@@ -200,24 +200,35 @@ private:
   typedef CycleDataReader<CDataCache> CDCacheReader;
   typedef CycleDataWriter<CDataCache> CDCacheWriter;
 
-public:  // It is not clear why MSVC7 needs this class to be public.  
+public:
+  // The CacheKey class separates out just the part of CacheEntry that
+  // is used to key the cache entry within the map.  We have this as a
+  // separate class so we can easily look up a new entry in the map,
+  // without having to execute the relatively expensive CacheEntry
+  // constructor.
+  class CacheKey {
+  public:
+    INLINE CacheKey(const GeomVertexFormat *modifier);
+    INLINE bool operator < (const CacheKey &other) const;
+
+    CPT(GeomVertexFormat) _modifier;
+  };
+  // It is not clear why MSVC7 needs this class to be public.  
   class CacheEntry : public GeomCacheEntry {
   public:
-    INLINE CacheEntry(const GeomVertexFormat *modifier);
     INLINE CacheEntry(GeomVertexData *source,
                       const GeomVertexFormat *modifier);
     ALLOC_DELETED_CHAIN(CacheEntry);
-    INLINE bool operator < (const CacheEntry &other) const;
 
     virtual void evict_callback();
     virtual void output(ostream &out) const;
 
-    GeomVertexData *_source;  // A back pointer to the containing GeomVertexData
-    CPT(GeomVertexFormat) _modifier;
+    GeomVertexData *_source;  // A back pointer to the containing data.
+    CacheKey _key;
 
     PipelineCycler<CDataCache> _cycler;
   };
-  typedef pset<PT(CacheEntry), IndirectLess<CacheEntry> > Cache;
+  typedef pmap<const CacheKey *, PT(CacheEntry), IndirectLess<CacheKey> > Cache;
 
 private:
   // This is the data that must be cycled between pipeline stages.