From 8078fa2b38d0b214c0eab98e8bbf7cfd400cc083 Mon Sep 17 00:00:00 2001 From: rdb Date: Tue, 17 Oct 2017 21:29:03 +0200 Subject: [PATCH] More efficiently store SimpleHashMap with empty values --- panda/src/pgraph/renderAttrib.cxx | 2 +- panda/src/pgraph/renderAttrib.h | 4 +- panda/src/pgraph/renderAttribRegistry.cxx | 2 +- panda/src/pgraph/renderState.cxx | 4 +- panda/src/pgraph/renderState.h | 4 +- panda/src/pgraph/transformState.cxx | 2 +- panda/src/pgraph/transformState.h | 4 +- panda/src/putil/simpleHashMap.I | 29 +++++++--- panda/src/putil/simpleHashMap.h | 70 +++++++++++++++++------ 9 files changed, 79 insertions(+), 42 deletions(-) diff --git a/panda/src/pgraph/renderAttrib.cxx b/panda/src/pgraph/renderAttrib.cxx index 37b65f3b2a..385d7ee740 100644 --- a/panda/src/pgraph/renderAttrib.cxx +++ b/panda/src/pgraph/renderAttrib.cxx @@ -389,7 +389,7 @@ return_unique(RenderAttrib *attrib) { // deleted while it's in it. attrib->ref(); } - si = _attribs->store(attrib, Empty()); + si = _attribs->store(attrib, nullptr); // Save the index and return the input attrib. attrib->_saved_entry = si; diff --git a/panda/src/pgraph/renderAttrib.h b/panda/src/pgraph/renderAttrib.h index aaa537ee5f..5bf54cf32f 100644 --- a/panda/src/pgraph/renderAttrib.h +++ b/panda/src/pgraph/renderAttrib.h @@ -185,9 +185,7 @@ public: private: // This mutex protects _attribs. static LightReMutex *_attribs_lock; - class Empty { - }; - typedef SimpleHashMap > Attribs; + typedef SimpleHashMap > Attribs; static Attribs *_attribs; int _saved_entry; diff --git a/panda/src/pgraph/renderAttribRegistry.cxx b/panda/src/pgraph/renderAttribRegistry.cxx index cd011e838d..4c27e4c0a7 100644 --- a/panda/src/pgraph/renderAttribRegistry.cxx +++ b/panda/src/pgraph/renderAttribRegistry.cxx @@ -93,7 +93,7 @@ register_slot(TypeHandle type_handle, int sort, RenderAttrib *default_attrib) { // If this attribute was already registered, something odd is going on. nassertr(RenderAttrib::_attribs->find(default_attrib) == -1, 0); default_attrib->_saved_entry = - RenderAttrib::_attribs->store(default_attrib, RenderAttrib::Empty()); + RenderAttrib::_attribs->store(default_attrib, nullptr); } // It effectively lives forever. Might as well make it official. diff --git a/panda/src/pgraph/renderState.cxx b/panda/src/pgraph/renderState.cxx index ee53a7d4a4..1aad1ed25c 100644 --- a/panda/src/pgraph/renderState.cxx +++ b/panda/src/pgraph/renderState.cxx @@ -1338,7 +1338,7 @@ return_unique(RenderState *state) { // deleted while it's in it. state->cache_ref(); } - si = _states->store(state, Empty()); + si = _states->store(state, nullptr); // Save the index and return the input state. state->_saved_entry = si; @@ -1865,7 +1865,7 @@ init_states() { // is declared globally, and lives forever. RenderState *state = new RenderState; state->local_object(); - state->_saved_entry = _states->store(state, Empty()); + state->_saved_entry = _states->store(state, nullptr); _empty_state = state; } diff --git a/panda/src/pgraph/renderState.h b/panda/src/pgraph/renderState.h index aa337bbb04..a7dc550625 100644 --- a/panda/src/pgraph/renderState.h +++ b/panda/src/pgraph/renderState.h @@ -227,9 +227,7 @@ private: // cache, which is encoded in _composition_cache and // _invert_composition_cache. static LightReMutex *_states_lock; - class Empty { - }; - typedef SimpleHashMap > States; + typedef SimpleHashMap > States; static States *_states; static const RenderState *_empty_state; diff --git a/panda/src/pgraph/transformState.cxx b/panda/src/pgraph/transformState.cxx index 3c28e358bf..bff497298e 100644 --- a/panda/src/pgraph/transformState.cxx +++ b/panda/src/pgraph/transformState.cxx @@ -1512,7 +1512,7 @@ return_unique(TransformState *state) { // deleted while it's in it. state->cache_ref(); } - si = _states->store(state, Empty()); + si = _states->store(state, nullptr); // Save the index and return the input state. state->_saved_entry = si; diff --git a/panda/src/pgraph/transformState.h b/panda/src/pgraph/transformState.h index b4efd5abf3..b770e7bf52 100644 --- a/panda/src/pgraph/transformState.h +++ b/panda/src/pgraph/transformState.h @@ -253,9 +253,7 @@ private: // cache, which is encoded in _composition_cache and // _invert_composition_cache. static LightReMutex *_states_lock; - class Empty { - }; - typedef SimpleHashMap > States; + typedef SimpleHashMap > States; static States *_states; static CPT(TransformState) _identity_state; static CPT(TransformState) _invalid_state; diff --git a/panda/src/putil/simpleHashMap.I b/panda/src/putil/simpleHashMap.I index 5f797dfde1..dc954bac97 100644 --- a/panda/src/putil/simpleHashMap.I +++ b/panda/src/putil/simpleHashMap.I @@ -128,7 +128,7 @@ store(const Key &key, const Value &data) { } if (is_element(index, key)) { // This element is already in the map; replace the data at that key. - _table[index]._data = data; + set_data(index, data); #ifdef _DEBUG nassertr(validate(), index); #endif @@ -151,7 +151,7 @@ store(const Key &key, const Value &data) { return index; } if (is_element(index, key)) { - _table[index]._data = data; + set_data(index, data); #ifdef _DEBUG nassertr(validate(), index); #endif @@ -200,8 +200,7 @@ remove(const Key &key) { // Swap it with the last one, so that we don't get any gaps in the table // of entries. - _table[index]._key = move(_table[last]._key); - _table[index]._data = move(_table[last]._data); + _table[index] = move(_table[last]); index_array[(size_t)other_slot] = index; } @@ -311,8 +310,8 @@ get_key(size_t n) const { template INLINE const Value &SimpleHashMap:: get_data(size_t n) const { - nassertr(n < _num_entries, _table[n]._data); - return _table[n]._data; + nassertr(n < _num_entries, _table[n].get_data()); + return _table[n].get_data(); } /** @@ -323,8 +322,8 @@ get_data(size_t n) const { template INLINE Value &SimpleHashMap:: modify_data(size_t n) { - nassertr(n < _num_entries, _table[n]._data); - return _table[n]._data; + nassertr(n < _num_entries, _table[n].modify_data()); + return _table[n].modify_data(); } /** @@ -336,7 +335,19 @@ template INLINE void SimpleHashMap:: set_data(size_t n, const Value &data) { nassertv(n < _num_entries); - _table[n]._data = data; + _table[n].set_data(data); +} + +/** + * Changes the data for the nth entry of the table. + * + * @param n should be in the range 0 <= n < size(). + */ +template +INLINE void SimpleHashMap:: +set_data(size_t n, Value &&data) { + nassertv(n < _num_entries); + _table[n].set_data(move(data)); } /** diff --git a/panda/src/putil/simpleHashMap.h b/panda/src/putil/simpleHashMap.h index d7d6aaa7fc..c7914e4206 100644 --- a/panda/src/putil/simpleHashMap.h +++ b/panda/src/putil/simpleHashMap.h @@ -18,6 +18,52 @@ #include "pvector.h" #include "config_util.h" +/** + * Entry in the SimpleHashMap. + */ +template +class SimpleKeyValuePair { +public: + INLINE SimpleKeyValuePair(const Key &key, const Value &data) : + _key(key), + _data(data) {} + + Key _key; + + ALWAYS_INLINE const Value &get_data() const { + return _data; + } + ALWAYS_INLINE Value &modify_data() { + return _data; + } + ALWAYS_INLINE void set_data(const Value &data) { + _data = data; + } + ALWAYS_INLINE void set_data(Value &&data) { + _data = move(data); + } + +private: + Value _data; +}; + +/** + * Specialisation of SimpleKeyValuePair to not waste memory for nullptr_t + * values. This allows effectively using SimpleHashMap as a set. + */ +template +class SimpleKeyValuePair { +public: + INLINE SimpleKeyValuePair(const Key &key, nullptr_t data) : + _key(key) {} + + Key _key; + + ALWAYS_INLINE_CONSTEXPR static nullptr_t get_data() { return nullptr; } + ALWAYS_INLINE_CONSTEXPR static nullptr_t modify_data() { return nullptr; } + ALWAYS_INLINE static void set_data(nullptr_t) {} +}; + /** * This template class implements an unordered map of keys to data, * implemented as a hashtable. It is similar to STL's hash_map, but @@ -28,6 +74,8 @@ * (d) it allows for efficient iteration over the entries, * (e) permits removal and resizing during forward iteration, and * (f) it has a constexpr constructor. + * + * It can also be used as a set, by using nullptr_t as Value typename. */ template > > class SimpleHashMap { @@ -55,6 +103,7 @@ public: INLINE const Value &get_data(size_t n) const; INLINE Value &modify_data(size_t n); INLINE void set_data(size_t n, const Value &data); + INLINE void set_data(size_t n, Value &&data); void remove_element(size_t n); INLINE size_t get_num_entries() const; @@ -67,8 +116,6 @@ public: INLINE bool consider_shrink_table(); private: - class TableEntry; - INLINE size_t get_hash(const Key &key) const; INLINE size_t next_hash(size_t hash) const; @@ -82,23 +129,8 @@ private: INLINE bool consider_expand_table(); void resize_table(size_t new_size); - class TableEntry { - public: - INLINE TableEntry(const Key &key, const Value &data) : - _key(key), - _data(data) {} - INLINE TableEntry(const TableEntry ©) : - _key(copy._key), - _data(copy._data) {} -#ifdef USE_MOVE_SEMANTICS - INLINE TableEntry(TableEntry &&from) NOEXCEPT : - _key(move(from._key)), - _data(move(from._data)) {} -#endif - Key _key; - Value _data; - }; - +public: + typedef SimpleKeyValuePair TableEntry; TableEntry *_table; DeletedBufferChain *_deleted_chain; size_t _table_size;