diff --git a/panda/src/framework/pandaFramework.cxx b/panda/src/framework/pandaFramework.cxx index bbb764ca68..a056cc6eb1 100644 --- a/panda/src/framework/pandaFramework.cxx +++ b/panda/src/framework/pandaFramework.cxx @@ -134,6 +134,11 @@ open_framework(int &argc, char **&argv) { _task_mgr.add(task); } + if (garbage_collect_states) { + PT(GenericAsyncTask) task = new GenericAsyncTask("garbage_collect", task_garbage_collect, this); + _task_mgr.add(task); + } + _highlight_wireframe = NodePath("wireframe"); _highlight_wireframe.set_render_mode_wireframe(1); _highlight_wireframe.set_texture_off(1); @@ -1619,3 +1624,15 @@ task_clear_text(GenericAsyncTask *task, void *data) { self->clear_text(); return AsyncTask::DS_cont; } + +//////////////////////////////////////////////////////////////////// +// Function: PandaFramework::task_garbage_collect +// Access: Public, Static +// Description: This task is created automatically if +// garbage_collect_states is true. It calls the needed +// TransformState::garbage_collect() method each frame. +//////////////////////////////////////////////////////////////////// +AsyncTask::DoneStatus PandaFramework:: +task_garbage_collect(GenericAsyncTask *task, void *data) { + TransformState::garbage_collect(); +} diff --git a/panda/src/framework/pandaFramework.h b/panda/src/framework/pandaFramework.h index 819bbef241..775e994b86 100644 --- a/panda/src/framework/pandaFramework.h +++ b/panda/src/framework/pandaFramework.h @@ -159,6 +159,7 @@ public: static AsyncTask::DoneStatus task_play_frame(GenericAsyncTask *task, void *data); static AsyncTask::DoneStatus task_clear_text(GenericAsyncTask *task, void *data); + static AsyncTask::DoneStatus task_garbage_collect(GenericAsyncTask *task, void *data); private: bool _is_open; diff --git a/panda/src/grutil/multitexReducer.I b/panda/src/grutil/multitexReducer.I index ac0d04dced..f1becbcd1e 100644 --- a/panda/src/grutil/multitexReducer.I +++ b/panda/src/grutil/multitexReducer.I @@ -75,7 +75,7 @@ operator < (const MultitexReducer::StageInfo &other) const { return _tex < other._tex; } if (_tex_mat != other._tex_mat) { - return _tex_mat->sorts_less(*other._tex_mat, true); + return _tex_mat->compare_to(*other._tex_mat, true) < 0; } return false; diff --git a/panda/src/pgraph/config_pgraph.cxx b/panda/src/pgraph/config_pgraph.cxx index 574405fb7c..76c0b9d2a3 100644 --- a/panda/src/pgraph/config_pgraph.cxx +++ b/panda/src/pgraph/config_pgraph.cxx @@ -196,6 +196,15 @@ ConfigVariableBool garbage_collect_states PRC_DESC("This temporary config variable is used for development only. " "Do not set!")); +ConfigVariableDouble garbage_collect_states_rate +("garbage-collect-states-rate", 0.25, + PRC_DESC("The fraction of the total number of TransformStates " + "(or RenderStates, or whatever) that are processed with " + "each garbage collection step. Setting this larger (up to " + "1.0) will ensure that more states are collected each frame, " + "limiting the wasted size of the cache, but will require more " + "processing time each frame.")); + ConfigVariableBool transform_cache ("transform-cache", true, PRC_DESC("Set this true to enable the cache of TransformState objects. " diff --git a/panda/src/pgraph/config_pgraph.h b/panda/src/pgraph/config_pgraph.h index 6d36f3782c..e1a7b050b1 100644 --- a/panda/src/pgraph/config_pgraph.h +++ b/panda/src/pgraph/config_pgraph.h @@ -44,7 +44,8 @@ extern ConfigVariableBool compose_componentwise; extern ConfigVariableBool uniquify_matrix; extern ConfigVariableBool paranoid_const; extern ConfigVariableBool auto_break_cycles; -extern ConfigVariableBool garbage_collect_states; +extern EXPCL_PANDA_PGRAPH ConfigVariableBool garbage_collect_states; +extern ConfigVariableDouble garbage_collect_states_rate; extern ConfigVariableBool transform_cache; extern ConfigVariableBool state_cache; extern ConfigVariableBool uniquify_transforms; diff --git a/panda/src/pgraph/renderAttrib.cxx b/panda/src/pgraph/renderAttrib.cxx index 02d9f0a0ab..69cde8e898 100644 --- a/panda/src/pgraph/renderAttrib.cxx +++ b/panda/src/pgraph/renderAttrib.cxx @@ -144,8 +144,7 @@ cull_callback(CullTraverser *, const CullTraverserData &) const { //////////////////////////////////////////////////////////////////// bool RenderAttrib:: unref() const { - // This is flawed, but this is development only. - if (garbage_collect_states) { + if (!state_cache) { return ReferenceCount::unref(); } @@ -295,11 +294,9 @@ CPT(RenderAttrib) RenderAttrib:: return_unique(RenderAttrib *attrib) { nassertr(attrib != (RenderAttrib *)NULL, attrib); -#ifndef NDEBUG if (!state_cache) { return attrib; } -#endif #ifndef NDEBUG if (paranoid_const) { diff --git a/panda/src/pgraph/renderState.cxx b/panda/src/pgraph/renderState.cxx index dec515beaf..b12469b1d2 100644 --- a/panda/src/pgraph/renderState.cxx +++ b/panda/src/pgraph/renderState.cxx @@ -702,8 +702,7 @@ adjust_all_priorities(int adjustment) const { //////////////////////////////////////////////////////////////////// bool RenderState:: unref() const { - // This is flawed, but this is development only. - if (garbage_collect_states) { + if (!state_cache) { return ReferenceCount::unref(); } diff --git a/panda/src/pgraph/transformState.I b/panda/src/pgraph/transformState.I index 3fd9e2cdb8..1c576af66a 100644 --- a/panda/src/pgraph/transformState.I +++ b/panda/src/pgraph/transformState.I @@ -19,14 +19,22 @@ // Description: Provides an arbitrary ordering among all unique // TransformStates, so we can store the essentially // different ones in a big set and throw away the rest. -// -// This is the same as sorts_less(), except the -// uniquify_matrix value is implicit from the Config.prc -// file. //////////////////////////////////////////////////////////////////// INLINE bool TransformState:: operator < (const TransformState &other) const { - return sorts_less(other, uniquify_matrix); + return compare_to(other) < 0; +} + +//////////////////////////////////////////////////////////////////// +// Function: TransformState::compare_to +// Access: Published +// Description: Provides an arbitrary ordering among all unique +// TransformStates, so we can store the essentially +// different ones in a big set and throw away the rest. +//////////////////////////////////////////////////////////////////// +INLINE int TransformState:: +compare_to(const TransformState &other) const { + return compare_to(other, uniquify_matrix); } //////////////////////////////////////////////////////////////////// diff --git a/panda/src/pgraph/transformState.cxx b/panda/src/pgraph/transformState.cxx index 90b52da6d0..0287225815 100644 --- a/panda/src/pgraph/transformState.cxx +++ b/panda/src/pgraph/transformState.cxx @@ -31,7 +31,10 @@ TransformState::States *TransformState::_states = NULL; CPT(TransformState) TransformState::_identity_state; CPT(TransformState) TransformState::_invalid_state; UpdateSeq TransformState::_last_cycle_detect; +int TransformState::_garbage_index = 0; + PStatCollector TransformState::_cache_update_pcollector("*:State Cache:Update"); +PStatCollector TransformState::_garbage_collect_pcollector("*:State Cache:Garbage Collect"); PStatCollector TransformState::_transform_compose_pcollector("*:State Cache:Compose Transform"); PStatCollector TransformState::_transform_invert_pcollector("*:State Cache:Invert Transform"); PStatCollector TransformState::_transform_calc_pcollector("*:State Cache:Calc Components"); @@ -58,7 +61,7 @@ TransformState() : _lock("TransformState") { if (_states == (States *)NULL) { init_states(); } - _saved_entry = _states->end(); + _saved_entry = -1; _flags = F_is_identity | F_singular_known | F_is_2d; _inv_mat = (LMatrix4f *)NULL; _cache_stats.add_num_states(1); @@ -105,7 +108,7 @@ TransformState:: LightReMutexHolder holder(*_states_lock); // unref() should have cleared these. - nassertv(_saved_entry == _states->end()); + nassertv(_saved_entry == -1); nassertv(_composition_cache.is_empty() && _invert_composition_cache.is_empty()); // If this was true at the beginning of the destructor, but is no @@ -115,7 +118,7 @@ TransformState:: } //////////////////////////////////////////////////////////////////// -// Function: TransformState::sorts_less +// Function: TransformState::compare_to // Access: Published // Description: Provides an arbitrary ordering among all unique // TransformStates, so we can store the essentially @@ -127,15 +130,15 @@ TransformState:: // TransformStates are uniquified, which is less // expensive. //////////////////////////////////////////////////////////////////// -bool TransformState:: -sorts_less(const TransformState &other, bool uniquify_matrix) const { +int TransformState:: +compare_to(const TransformState &other, bool uniquify_matrix) const { static const int significant_flags = (F_is_invalid | F_is_identity | F_components_given | F_hpr_given | F_quat_given | F_is_2d); int flags = (_flags & significant_flags); int other_flags = (other._flags & significant_flags); if (flags != other_flags) { - return flags < other_flags; + return flags < other_flags ? -1 : 1; } if ((_flags & (F_is_invalid | F_is_identity)) != 0) { @@ -149,39 +152,42 @@ sorts_less(const TransformState &other, bool uniquify_matrix) const { // componentwise. int c = _pos.compare_to(other._pos); if (c != 0) { - return c < 0; + return c; } if ((_flags & F_hpr_given) != 0) { c = _hpr.compare_to(other._hpr); if (c != 0) { - return c < 0; + return c; } } else if ((_flags & F_quat_given) != 0) { c = _quat.compare_to(other._quat); if (c != 0) { - return c < 0; + return c; } } c = _scale.compare_to(other._scale); if (c != 0) { - return c < 0; + return c; } c = _shear.compare_to(other._shear); - return c < 0; + return c; } // Otherwise, compare the matrices . . . if (uniquify_matrix) { // . . . but only if the user thinks that's a worthwhile // comparison. - return get_mat() < other.get_mat(); + return get_mat().compare_to(other.get_mat()); } else { // If not, we just compare the pointers. - return (this < &other); + if (this != &other) { + return (this < &other) ? -1 : 1; + } + return 0; } } @@ -711,13 +717,30 @@ invert_compose(const TransformState *other) const { //////////////////////////////////////////////////////////////////// bool TransformState:: unref() const { - // This is flawed, but this is development only. - if (garbage_collect_states) { + if (!transform_cache) { + // If we're not using the cache anyway, just allow the pointer to + // unref normally. return ReferenceCount::unref(); } + if (garbage_collect_states) { + // In the garbage collector case, we don't delete TransformStates + // immediately; instead, we allow them to remain in the cache with + // a ref count of 0, and we delete them later in + // garbage_collect(). + + ReferenceCount::unref(); + // Return true so that it is never deleted here. + return true; + } + + // Here is the normal refcounting case, with a normal cache, and + // without garbage collection in effect. + // We always have to grab the lock, since we will definitely need to // be holding it if we happen to drop the reference count to 0. + // Having to grab the lock at every call to unref() is a big + // limiting factor on parallelization. LightReMutexHolder holder(*_states_lock); if (auto_break_cycles && uniquify_transforms) { @@ -727,30 +750,7 @@ unref() const { // cache, leaving only references in the cache, then we need to // check for a cycle involving this TransformState and break it if // it exists. - - PStatTimer timer(_transform_break_cycles_pcollector); - - ++_last_cycle_detect; - if (r_detect_cycles(this, this, 1, _last_cycle_detect, NULL)) { - // Ok, we have a cycle. This will be a leak unless we break the - // cycle by freeing the cache on this object. - if (pgraph_cat.is_debug()) { - pgraph_cat.debug() - << "Breaking cycle involving " << (*this) << "\n"; - } - - ((TransformState *)this)->remove_cache_pointers(); - } else { - ++_last_cycle_detect; - if (r_detect_reverse_cycles(this, this, 1, _last_cycle_detect, NULL)) { - if (pgraph_cat.is_debug()) { - pgraph_cat.debug() - << "Breaking cycle involving " << (*this) << "\n"; - } - - ((TransformState *)this)->remove_cache_pointers(); - } - } + ((TransformState *)this)->detect_and_break_cycles(); } } @@ -1017,7 +1017,7 @@ get_num_states() { return 0; } LightReMutexHolder holder(*_states_lock); - return _states->size(); + return _states->get_num_entries(); } //////////////////////////////////////////////////////////////////// @@ -1051,9 +1051,12 @@ get_num_unused_states() { typedef pmap StateCount; StateCount state_count; - States::iterator si; - for (si = _states->begin(); si != _states->end(); ++si) { - const TransformState *state = (*si); + int size = _states->get_size(); + for (int si = 0; si < size; ++si) { + if (!_states->has_element(si)) { + continue; + } + const TransformState *state = _states->get_key(si); int i; int cache_size = state->_composition_cache.get_size(); @@ -1142,7 +1145,7 @@ clear_cache() { LightReMutexHolder holder(*_states_lock); PStatTimer timer(_cache_update_pcollector); - int orig_size = _states->size(); + int orig_size = _states->get_num_entries(); // First, we need to copy the entire set of states to a temporary // vector, reference-counting each object. That way we can walk @@ -1153,8 +1156,14 @@ clear_cache() { TempStates temp_states; temp_states.reserve(orig_size); - copy(_states->begin(), _states->end(), - back_inserter(temp_states)); + int size = _states->get_size(); + for (int si = 0; si < size; ++si) { + if (!_states->has_element(si)) { + continue; + } + const TransformState *state = _states->get_key(si); + temp_states.push_back(state); + } // Now it's safe to walk through the list, destroying the cache // within each object as we go. Nothing will be destructed till @@ -1196,7 +1205,73 @@ clear_cache() { // held only within the various objects' caches will go away. } - int new_size = _states->size(); + int new_size = _states->get_num_entries(); + return orig_size - new_size; +} + +//////////////////////////////////////////////////////////////////// +// Function: TransformState::garbage_collect +// Access: Published, Static +// Description: Performs a garbage-collection cycle. This must be +// called periodically if garbage-collect-states is true +// to ensure that TransformStates get cleaned up +// appropriately. It does no harm to call it even if +// this variable is not true, but there is probably no +// advantage in that case. +//////////////////////////////////////////////////////////////////// +int TransformState:: +garbage_collect() { + if (_states == (States *)NULL) { + return 0; + } + LightReMutexHolder holder(*_states_lock); + + PStatTimer timer(_garbage_collect_pcollector); + int orig_size = _states->get_num_entries(); + + // How many elements to process this pass? + int size = _states->get_size(); + int num_this_pass = int(size * garbage_collect_states_rate); + if (num_this_pass <= 0) { + return 0; + } + num_this_pass = min(num_this_pass, size); + int stop_at_element = (_garbage_index + num_this_pass) % size; + + int num_elements = 0; + int si = _garbage_index; + do { + if (_states->has_element(si)) { + ++num_elements; + TransformState *state = (TransformState *)_states->get_key(si); + if (auto_break_cycles && uniquify_transforms) { + if (state->get_cache_ref_count() > 0 && + state->get_ref_count() == state->get_cache_ref_count()) { + // If we have removed all the references to this state not in + // the cache, leaving only references in the cache, then we + // need to check for a cycle involving this TransformState and + // break it if it exists. + state->detect_and_break_cycles(); + } + } + + if (state->get_ref_count() == 0) { + // This state has recently been unreffed to 0, but it hasn't + // been deleted yet (because we have overloaded unref(), + // above, to always return true). Now it's time to delete it. + // This is safe, because we're holding the _states_lock, so + // it's not possible for some other thread to find the state + // in the cache and ref it while we're doing this. + state->release_new(); + state->remove_cache_pointers(); + delete state; + } + } + ++si; + } while (si != stop_at_element); + _garbage_index = si; + + int new_size = _states->get_num_entries(); return orig_size - new_size; } @@ -1229,9 +1304,12 @@ list_cycles(ostream &out) { VisitedStates visited; CompositionCycleDesc cycle_desc; - States::iterator si; - for (si = _states->begin(); si != _states->end(); ++si) { - const TransformState *state = (*si); + int size = _states->get_size(); + for (int si = 0; si < size; ++si) { + if (!_states->has_element(si)) { + continue; + } + const TransformState *state = _states->get_key(si); bool inserted = visited.insert(state).second; if (inserted) { @@ -1305,10 +1383,14 @@ list_states(ostream &out) { } LightReMutexHolder holder(*_states_lock); - out << _states->size() << " states:\n"; - States::const_iterator si; - for (si = _states->begin(); si != _states->end(); ++si) { - const TransformState *state = (*si); + out << _states->get_num_entries() << " states:\n"; + + int size = _states->get_size(); + for (int si = 0; si < size; ++si) { + if (!_states->has_element(si)) { + continue; + } + const TransformState *state = _states->get_key(si); state->write(out, 2); } } @@ -1332,36 +1414,51 @@ validate_states() { PStatTimer timer(_transform_validate_pcollector); LightReMutexHolder holder(*_states_lock); - if (_states->empty()) { + if (_states->is_empty()) { return true; } - States::const_iterator si = _states->begin(); - States::const_iterator snext = si; - ++snext; - nassertr((*si)->get_ref_count() > 0, false); - while (snext != _states->end()) { - if (!(*(*si) < *(*snext))) { + int size = _states->get_size(); + int si = 0; + while (si < size && !_states->has_element(si)) { + ++si; + } + nassertr(si < size, false); + nassertr(_states->get_key(si)->get_ref_count() > 0, false); + int snext = si; + while (snext < size && !_states->has_element(snext)) { + ++snext; + } + while (snext < size) { + const TransformState *ssi = _states->get_key(si); + const TransformState *ssnext = _states->get_key(snext); + int c = ssi->compare_to(*ssnext); + if (c >= 0) { pgraph_cat.error() << "TransformStates out of order!\n"; - (*si)->write(pgraph_cat.error(false), 2); - (*snext)->write(pgraph_cat.error(false), 2); + ssi->write(pgraph_cat.error(false), 2); + ssnext->write(pgraph_cat.error(false), 2); return false; } - if ((*(*snext) < *(*si))) { + int ci = ssnext->compare_to(*ssi); + if ((ci < 0) != (c > 0) || + (ci > 0) != (c < 0) || + (ci == 0) != (c == 0)) { pgraph_cat.error() - << "TransformState::operator < not defined properly!\n"; + << "TransformState::compare_to() not defined properly!\n"; pgraph_cat.error(false) - << "a < b: " << (*(*si) < *(*snext)) << "\n"; + << "(a, b): " << c << "\n"; pgraph_cat.error(false) - << "b < a: " << (*(*snext) < *(*si)) << "\n"; - (*si)->write(pgraph_cat.error(false), 2); - (*snext)->write(pgraph_cat.error(false), 2); + << "(b, a): " << ci << "\n"; + ssi->write(pgraph_cat.error(false), 2); + ssnext->write(pgraph_cat.error(false), 2); return false; } si = snext; - ++snext; - nassertr((*si)->get_ref_count() > 0, false); + while (snext < size && !_states->has_element(snext)) { + ++snext; + } + nassertr(_states->get_key(si)->get_ref_count() > 0, false); } return true; @@ -1383,18 +1480,23 @@ get_states() { } LightReMutexHolder holder(*_states_lock); - size_t num_states = _states->size(); + size_t num_states = _states->get_num_entries(); PyObject *list = PyList_New(num_states); - States::const_iterator si; - size_t i; - for (si = _states->begin(), i = 0; si != _states->end(); ++si, ++i) { - nassertr(i < num_states, list); - const TransformState *state = (*si); + size_t i = 0; + + int size = _states->get_size(); + for (int si = 0; si < size; ++si) { + if (!_states->has_element(si)) { + continue; + } + const TransformState *state = _states->get_key(si); state->ref(); PyObject *a = DTool_CreatePyInstanceTyped((void *)state, Dtool_TransformState, true, true, state->get_type_index()); + nassertr(i < num_states, list); PyList_SET_ITEM(list, i, a); + ++i; } nassertr(i == num_states, list); return list; @@ -1475,7 +1577,7 @@ return_unique(TransformState *state) { LightReMutexHolder holder(*_states_lock); - if (state->_saved_entry != _states->end()) { + if (state->_saved_entry != -1) { // This state is already in the cache. nassertr(_states->find(state) == state->_saved_entry, state); return state; @@ -1485,17 +1587,18 @@ return_unique(TransformState *state) { // the end of this function if no one else uses it. CPT(TransformState) pt_state = state; - pair result = _states->insert(state); - if (result.second) { - // The state was inserted; save the iterator and return the - // input state. - state->_saved_entry = result.first; - return pt_state; + int si = _states->find(state); + if (si != -1) { + // There's an equivalent state already in the set. Return it. + return _states->get_key(si); } - // The state was not inserted; there must be an equivalent one - // already in the set. Return that one. - return *(result.first); + // Not already in the set; add it. + si = _states->store(state, Empty()); + + // Save the index and return the input state. + state->_saved_entry = si; + return pt_state; } //////////////////////////////////////////////////////////////////// @@ -1862,6 +1965,40 @@ do_invert_compose(const TransformState *other) const { } } +//////////////////////////////////////////////////////////////////// +// Function: TransformState::detect_and_break_cycles +// Access: Private +// Description: Detects whether there is a cycle in the cache that +// begins with this state. If any are detected, breaks +// them by removing this state from the cache. +//////////////////////////////////////////////////////////////////// +void TransformState:: +detect_and_break_cycles() { + PStatTimer timer(_transform_break_cycles_pcollector); + + ++_last_cycle_detect; + if (r_detect_cycles(this, this, 1, _last_cycle_detect, NULL)) { + // Ok, we have a cycle. This will be a leak unless we break the + // cycle by freeing the cache on this object. + if (pgraph_cat.is_debug()) { + pgraph_cat.debug() + << "Breaking cycle involving " << (*this) << "\n"; + } + + remove_cache_pointers(); + } else { + ++_last_cycle_detect; + if (r_detect_reverse_cycles(this, this, 1, _last_cycle_detect, NULL)) { + if (pgraph_cat.is_debug()) { + pgraph_cat.debug() + << "Breaking cycle involving " << (*this) << "\n"; + } + + remove_cache_pointers(); + } + } +} + //////////////////////////////////////////////////////////////////// // Function: TransformState::r_detect_cycles // Access: Private, Static @@ -2025,10 +2162,11 @@ void TransformState:: release_new() { nassertv(_states_lock->debug_is_locked()); - if (_saved_entry != _states->end()) { - nassertv(_states->find(this) == _saved_entry); - _states->erase(_saved_entry); - _saved_entry = _states->end(); + if (_saved_entry != -1) { + //nassertv(_states->find(this) == _saved_entry); + _saved_entry = _states->find(this); + _states->remove_element(_saved_entry); + _saved_entry = -1; } } @@ -2197,11 +2335,18 @@ do_calc_hash() { _hash = _shear.add_hash(_hash); } else { - // Otherwise, hash the pointer only--any two different - // matrix-based TransformStates are considered to be different, - // even if their matrices have the same values. + // Otherwise, hash the matrix . . . + if (uniquify_matrix) { + // . . . but only if the user thinks that's worthwhile. + _hash = get_mat().add_hash(_hash); - _hash = pointer_hash::add_hash(_hash, this); + } else { + // Otherwise, hash the pointer only--any two different + // matrix-based TransformStates are considered to be different, + // even if their matrices have the same values. + + _hash = pointer_hash::add_hash(_hash, this); + } } } diff --git a/panda/src/pgraph/transformState.h b/panda/src/pgraph/transformState.h index 943c7939bd..1575aba3fd 100644 --- a/panda/src/pgraph/transformState.h +++ b/panda/src/pgraph/transformState.h @@ -73,7 +73,8 @@ public: PUBLISHED: INLINE bool operator < (const TransformState &other) const; - bool sorts_less(const TransformState &other, bool uniquify_matrix) const; + INLINE int compare_to(const TransformState &other) const; + int compare_to(const TransformState &other, bool uniquify_matrix) const; INLINE size_t get_hash() const; static CPT(TransformState) make_identity(); @@ -201,6 +202,7 @@ PUBLISHED: static int get_num_states(); static int get_num_unused_states(); static int clear_cache(); + static int garbage_collect(); static void list_cycles(ostream &out); static void list_states(ostream &out); static bool validate_states(); @@ -237,6 +239,7 @@ private: CPT(TransformState) store_compose(const TransformState *other, const TransformState *result); CPT(TransformState) do_invert_compose(const TransformState *other) const; CPT(TransformState) store_invert_compose(const TransformState *other, const TransformState *result); + void detect_and_break_cycles(); static bool r_detect_cycles(const TransformState *start_state, const TransformState *current_state, int length, UpdateSeq this_seq, @@ -254,15 +257,17 @@ private: // to the cache, which is encoded in _composition_cache and // _invert_composition_cache. static LightReMutex *_states_lock; - typedef phash_set > States; + class Empty { + }; + typedef SimpleHashMap > States; static States *_states; static CPT(TransformState) _identity_state; static CPT(TransformState) _invalid_state; - // This iterator records the entry corresponding to this TransformState - // object in the above global set. We keep the iterator around so - // we can remove it when the TransformState destructs. - States::iterator _saved_entry; + // This iterator records the entry corresponding to this + // TransformState object in the above global set. We keep the index + // around so we can remove it when the TransformState destructs. + int _saved_entry; // This data structure manages the job of caching the composition of // two TransformStates. It's complicated because we have to be sure to @@ -292,7 +297,12 @@ private: UpdateSeq _cycle_detect; static UpdateSeq _last_cycle_detect; + // This keeps track of our current position through the garbage + // collection cycle. + static int _garbage_index; + static PStatCollector _cache_update_pcollector; + static PStatCollector _garbage_collect_pcollector; static PStatCollector _transform_compose_pcollector; static PStatCollector _transform_invert_pcollector; static PStatCollector _transform_calc_pcollector; diff --git a/panda/src/putil/simpleHashMap.I b/panda/src/putil/simpleHashMap.I index 43ff5c73fc..1ae33ada2a 100644 --- a/panda/src/putil/simpleHashMap.I +++ b/panda/src/putil/simpleHashMap.I @@ -111,19 +111,19 @@ find(const Key &key) const { // Access: Public // Description: Records the indicated key/data pair in the map. If // the key was already present, silently replaces it. -// Returns a reference to the value in the map. +// Returns the index at which it was stored. //////////////////////////////////////////////////////////////////// template -Value &SimpleHashMap:: +int SimpleHashMap:: store(const Key &key, const Value &data) { if (_table_size == 0) { // Special case: the first key in an empty table. - nassertr(_num_entries == 0, _table[0]._data); + nassertr(_num_entries == 0, -1); new_table(); size_t index = get_hash(key); store_new_element(index, key, data); ++_num_entries; - return _table[index]._data; + return index; } size_t index = get_hash(key); @@ -133,11 +133,11 @@ store(const Key &key, const Value &data) { } store_new_element(index, key, data); ++_num_entries; - return _table[index]._data; + return index; } if (is_element(index, key)) { _table[index]._data = data; - return _table[index]._data; + return index; } // There was some other key at the hashed slot. That's a hash @@ -151,19 +151,19 @@ store(const Key &key, const Value &data) { } store_new_element(i, key, data); ++_num_entries; - return _table[i]._data; + return i; } if (is_element(i, key)) { _table[i]._data = data; - return _table[i]._data; + return i; } i = (i + 1) & (_table_size - 1); } // Shouldn't get here unless _num_entries == _table_size, which // shouldn't be possible due to consider_expand_table(). - nassertr(false, _table[0]._data); - return _table[0]._data; // To satisfy compiler + nassertr(false, -1); + return -1; // To satisfy compiler } //////////////////////////////////////////////////////////////////// @@ -218,10 +218,10 @@ template INLINE Value &SimpleHashMap:: operator [] (const Key &key) { int index = find(key); - if (index != -1) { - return modify_data(index); + if (index == -1) { + index = store(key, Value()); } - return store(key, Value()); + return modify_data(index); } //////////////////////////////////////////////////////////////////// diff --git a/panda/src/putil/simpleHashMap.h b/panda/src/putil/simpleHashMap.h index b7039ef3da..9f72450ee5 100644 --- a/panda/src/putil/simpleHashMap.h +++ b/panda/src/putil/simpleHashMap.h @@ -39,7 +39,7 @@ public: INLINE void swap(SimpleHashMap &other); int find(const Key &key) const; - Value &store(const Key &key, const Value &data); + int store(const Key &key, const Value &data); INLINE bool remove(const Key &key); void clear();