From f410cff0cd252def9a6723facdd6f74fb04cef2c Mon Sep 17 00:00:00 2001 From: David Rose Date: Sun, 30 Apr 2006 20:24:14 +0000 Subject: [PATCH] separate out CacheKey and CacheEntry --- panda/src/gobj/geom.I | 58 +++++++++++++------------------ panda/src/gobj/geom.cxx | 10 +++--- panda/src/gobj/geom.h | 26 ++++++++++---- panda/src/gobj/geomMunger.cxx | 9 +++-- panda/src/gobj/geomVertexData.I | 42 +++++++++------------- panda/src/gobj/geomVertexData.cxx | 19 +++++----- panda/src/gobj/geomVertexData.h | 23 ++++++++---- 7 files changed, 94 insertions(+), 93 deletions(-) diff --git a/panda/src/gobj/geom.I b/panda/src/gobj/geom.I index 1e56ac25cd..d2e6246553 100644 --- a/panda/src/gobj/geom.I +++ b/panda/src/gobj/geom.I @@ -373,26 +373,36 @@ set_result(const Geom *geom_result, const GeomVertexData *data_result) { } //////////////////////////////////////////////////////////////////// -// Function: Geom::CacheEntry::Constructor +// Function: Geom::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 Geom::CacheEntry:: -CacheEntry(const GeomVertexData *source_data, const GeomMunger *modifier) : - _source(NULL), +INLINE Geom::CacheKey:: +CacheKey(const GeomVertexData *source_data, const GeomMunger *modifier) : _source_data(source_data), _modifier(modifier) { } +//////////////////////////////////////////////////////////////////// +// Function: Geom::CacheKey::operator < +// Access: Public +// Description: Provides a unique ordering within the map. +//////////////////////////////////////////////////////////////////// +INLINE bool Geom::CacheKey:: +operator < (const CacheKey &other) const { + if (_modifier != other._modifier) { + int compare = _modifier->geom_compare_to(*other._modifier); + if (compare != 0) { + return (compare < 0); + } + } + if (_source_data != other._source_data) { + return (_source_data < other._source_data); + } + return 0; +} + //////////////////////////////////////////////////////////////////// // Function: Geom::CacheEntry::Constructor // Access: Public @@ -402,30 +412,10 @@ INLINE Geom::CacheEntry:: CacheEntry(Geom *source, const GeomVertexData *source_data, const GeomMunger *modifier) : _source(source), - _source_data(source_data), - _modifier(modifier) + _key(source_data, modifier) { } -//////////////////////////////////////////////////////////////////// -// Function: Geom::CacheEntry::operator < -// Access: Public -// Description: Provides a unique ordering within the set. -//////////////////////////////////////////////////////////////////// -INLINE bool Geom::CacheEntry:: -operator < (const CacheEntry &other) const { - if (_modifier != other._modifier) { - int compare = _modifier->geom_compare_to(*other._modifier); - if (compare != 0) { - return (compare < 0); - } - } - if (_source_data != other._source_data) { - return (_source_data < other._source_data); - } - return 0; -} - //////////////////////////////////////////////////////////////////// // Function: Geom::CData::Constructor diff --git a/panda/src/gobj/geom.cxx b/panda/src/gobj/geom.cxx index 3220a72d33..03c51546d3 100644 --- a/panda/src/gobj/geom.cxx +++ b/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; } diff --git a/panda/src/gobj/geom.h b/panda/src/gobj/geom.h index 38293ea23f..7575c52dea 100644 --- a/panda/src/gobj/geom.h +++ b/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 CDCacheReader; typedef CycleDataWriter 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 _cycler; }; - typedef pset > Cache; + typedef pmap > Cache; private: // This is the data that must be cycled between pipeline stages. diff --git a/panda/src/gobj/geomMunger.cxx b/panda/src/gobj/geomMunger.cxx index 1f76189e95..1a7148d80d 100644 --- a/panda/src/gobj/geomMunger.cxx +++ b/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. diff --git a/panda/src/gobj/geomVertexData.I b/panda/src/gobj/geomVertexData.I index 8f758131c4..d9f2ea1a04 100644 --- a/panda/src/gobj/geomVertexData.I +++ b/panda/src/gobj/geomVertexData.I @@ -403,48 +403,38 @@ CDataCache(const GeomVertexData::CDataCache ©) : } //////////////////////////////////////////////////////////////////// -// 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::CacheKey::operator < +// Access: Public +// Description: Provides a unique ordering within the set. +//////////////////////////////////////////////////////////////////// +INLINE bool GeomVertexData::CacheKey:: +operator < (const CacheKey &other) const { + return _modifier < other._modifier; +} + //////////////////////////////////////////////////////////////////// // Function: GeomVertexData::CacheEntry::Constructor // Access: Public // Description: //////////////////////////////////////////////////////////////////// INLINE GeomVertexData::CacheEntry:: -CacheEntry(GeomVertexData *source, - const GeomVertexFormat *modifier) : +CacheEntry(GeomVertexData *source, const GeomVertexFormat *modifier) : _source(source), - _modifier(modifier) + _key(modifier) { } -//////////////////////////////////////////////////////////////////// -// Function: GeomVertexData::CacheEntry::operator < -// Access: Public -// Description: Provides a unique ordering within the set. -//////////////////////////////////////////////////////////////////// -INLINE bool GeomVertexData::CacheEntry:: -operator < (const CacheEntry &other) const { - return _modifier < other._modifier; -} - //////////////////////////////////////////////////////////////////// // Function: GeomVertexData::CData::Constructor // Access: Public diff --git a/panda/src/gobj/geomVertexData.cxx b/panda/src/gobj/geomVertexData.cxx index aa6fb1e968..46b4cd637c 100644 --- a/panda/src/gobj/geomVertexData.cxx +++ b/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; } //////////////////////////////////////////////////////////////////// diff --git a/panda/src/gobj/geomVertexData.h b/panda/src/gobj/geomVertexData.h index 1e5f6c6071..279dcf03e5 100644 --- a/panda/src/gobj/geomVertexData.h +++ b/panda/src/gobj/geomVertexData.h @@ -200,24 +200,35 @@ private: typedef CycleDataReader CDCacheReader; typedef CycleDataWriter 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 _cycler; }; - typedef pset > Cache; + typedef pmap > Cache; private: // This is the data that must be cycled between pipeline stages.