Change SimpleHashMap implementation to improve performance of GC.

The new implementation stores elements contiguously and keeps a separate sparse array of indices, similar to the new PyPy and CPython implementations.  This vastly improves performance when iterating over the individual elements, such as when garbage collecting states, and decreases memory usage.

panda/src/pgraph/renderAttrib.cxx

@@ -194,12 +194,9 @@ void RenderAttrib::
 list_attribs(ostream &out) {
   LightReMutexHolder holder(*_attribs_lock);
 
-  out << _attribs->get_num_entries() << " attribs:\n";
+  size_t size = _attribs->get_num_entries();
-  size_t size = _attribs->get_size();
+  out << size << " attribs:\n";
   for (size_t si = 0; si < size; ++si) {
-    if (!_attribs->has_element(si)) {
-      continue;
-    }
     const RenderAttrib *attrib = _attribs->get_key(si);
     attrib->write(out, 2);
   }
@@ -217,24 +214,28 @@ garbage_collect() {
   LightReMutexHolder holder(*_attribs_lock);
 
   PStatTimer timer(_garbage_collect_pcollector);
-  int orig_size = _attribs->get_num_entries();
+  size_t orig_size = _attribs->get_num_entries();
 
 #ifdef _DEBUG
   nassertr(_attribs->validate(), 0);
 #endif
 
   // How many elements to process this pass?
-  int size = _attribs->get_size();
+  size_t size = orig_size;
-  int num_this_pass = int(size * garbage_collect_states_rate);
+  size_t num_this_pass = max(0, 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;
 
   size_t si = _garbage_index;
+  if (si >= size) {
+    si = 0;
+  }
+
+  num_this_pass = min(num_this_pass, size);
+  size_t stop_at_element = (_garbage_index + num_this_pass) % size;
+
   do {
-    if (_attribs->has_element(si)) {
     RenderAttrib *attrib = (RenderAttrib *)_attribs->get_key(si);
     if (attrib->get_ref_count() == 1) {
       // This attrib has recently been unreffed to 1 (the one we added when
@@ -244,13 +245,20 @@ garbage_collect() {
       // while we're doing this.
       attrib->release_new();
       unref_delete(attrib);
-      }
+
+      // When we removed it from the hash map, it swapped the last element
+      // with the one we just removed.  So the current index contains one we
+      // still need to visit.
+      --size;
+      --si;
     }
 
     si = (si + 1) % size;
   } while (si != stop_at_element);
   _garbage_index = si;
 
+  nassertr(_attribs->get_num_entries() == size, 0);
+
 #ifdef _DEBUG
   nassertr(_attribs->validate(), 0);
 #endif
@@ -259,8 +267,7 @@ garbage_collect() {
   // size.  This will help reduce iteration overhead in the future.
   _attribs->consider_shrink_table();
 
-  size_t new_size = _attribs->get_num_entries();
+  return (int)orig_size - (int)size;
-  return orig_size - new_size;
 }
 
 /**
@@ -279,31 +286,22 @@ validate_attribs() {
     pgraph_cat.error()
       << "RenderAttrib::_attribs cache is invalid!\n";
 
-    size_t size = _attribs->get_size();
+    size_t size = _attribs->get_num_entries();
     for (size_t si = 0; si < size; ++si) {
-      if (!_attribs->has_element(si)) {
-        continue;
-      }
       const RenderAttrib *attrib = _attribs->get_key(si);
-      cerr << si << ": " << attrib << "\n";
+      //cerr << si << ": " << attrib << "\n";
       attrib->write(cerr, 2);
     }
 
     return false;
   }
 
-  size_t size = _attribs->get_size();
+  size_t size = _attribs->get_num_entries();
   size_t si = 0;
-  while (si < size && !_attribs->has_element(si)) {
-    ++si;
-  }
   nassertr(si < size, false);
   nassertr(_attribs->get_key(si)->get_ref_count() >= 0, false);
   size_t snext = si;
   ++snext;
-  while (snext < size && !_attribs->has_element(snext)) {
-    ++snext;
-  }
   while (snext < size) {
     nassertr(_attribs->get_key(snext)->get_ref_count() >= 0, false);
     const RenderAttrib *ssi = _attribs->get_key(si);
@@ -325,9 +323,6 @@ validate_attribs() {
     }
     si = snext;
     ++snext;
-    while (snext < size && !_attribs->has_element(snext)) {
-      ++snext;
-    }
   }
 
   return true;
@@ -518,10 +513,8 @@ release_new() {
   nassertv(_attribs_lock->debug_is_locked());
 
   if (_saved_entry != -1) {
-    // nassertv(_attribs->find(this) == _saved_entry);
-    _saved_entry = _attribs->find(this);
-    _attribs->remove_element(_saved_entry);
     _saved_entry = -1;
+    nassertv(_attribs->remove(this));
   }
 }
 

panda/src/pgraph/renderState.I

@@ -237,7 +237,7 @@ get_invert_composition_cache_num_entries() const {
 INLINE size_t RenderState::
 get_composition_cache_size() const {
   LightReMutexHolder holder(*_states_lock);
-  return _composition_cache.get_size();
+  return _composition_cache.get_num_entries();
 }
 
 /**
@@ -251,9 +251,6 @@ get_composition_cache_size() const {
 INLINE const RenderState *RenderState::
 get_composition_cache_source(size_t n) const {
   LightReMutexHolder holder(*_states_lock);
-  if (!_composition_cache.has_element(n)) {
-    return NULL;
-  }
   return _composition_cache.get_key(n);
 }
 
@@ -270,9 +267,6 @@ get_composition_cache_source(size_t n) const {
 INLINE const RenderState *RenderState::
 get_composition_cache_result(size_t n) const {
   LightReMutexHolder holder(*_states_lock);
-  if (!_composition_cache.has_element(n)) {
-    return NULL;
-  }
   return _composition_cache.get_data(n)._result;
 }
 
@@ -288,7 +282,7 @@ get_composition_cache_result(size_t n) const {
 INLINE size_t RenderState::
 get_invert_composition_cache_size() const {
   LightReMutexHolder holder(*_states_lock);
-  return _invert_composition_cache.get_size();
+  return _invert_composition_cache.get_num_entries();
 }
 
 /**
@@ -302,9 +296,6 @@ get_invert_composition_cache_size() const {
 INLINE const RenderState *RenderState::
 get_invert_composition_cache_source(size_t n) const {
   LightReMutexHolder holder(*_states_lock);
-  if (!_invert_composition_cache.has_element(n)) {
-    return NULL;
-  }
   return _invert_composition_cache.get_key(n);
 }
 
@@ -322,9 +313,6 @@ get_invert_composition_cache_source(size_t n) const {
 INLINE const RenderState *RenderState::
 get_invert_composition_cache_result(size_t n) const {
   LightReMutexHolder holder(*_states_lock);
-  if (!_invert_composition_cache.has_element(n)) {
-    return NULL;
-  }
   return _invert_composition_cache.get_data(n)._result;
 }
 

panda/src/pgraph/renderState.cxx

@@ -408,13 +408,13 @@ compose(const RenderState *other) const {
   CPT(RenderState) result = do_compose(other);
 
   _cache_stats.add_total_size(1);
-  _cache_stats.inc_adds(_composition_cache.get_size() == 0);
+  _cache_stats.inc_adds(_composition_cache.is_empty());
 
   ((RenderState *)this)->_composition_cache[other]._result = result;
 
   if (other != this) {
     _cache_stats.add_total_size(1);
-    _cache_stats.inc_adds(other->_composition_cache.get_size() == 0);
+    _cache_stats.inc_adds(other->_composition_cache.is_empty());
     ((RenderState *)other)->_composition_cache[this]._result = NULL;
   }
 
@@ -497,12 +497,12 @@ invert_compose(const RenderState *other) const {
   CPT(RenderState) result = do_invert_compose(other);
 
   _cache_stats.add_total_size(1);
-  _cache_stats.inc_adds(_invert_composition_cache.get_size() == 0);
+  _cache_stats.inc_adds(_invert_composition_cache.is_empty());
   ((RenderState *)this)->_invert_composition_cache[other]._result = result;
 
   if (other != this) {
     _cache_stats.add_total_size(1);
-    _cache_stats.inc_adds(other->_invert_composition_cache.get_size() == 0);
+    _cache_stats.inc_adds(other->_invert_composition_cache.is_empty());
     ((RenderState *)other)->_invert_composition_cache[this]._result = NULL;
   }
 
@@ -782,17 +782,13 @@ get_num_unused_states() {
   typedef pmap<const RenderState *, int> StateCount;
   StateCount state_count;
 
-  size_t size = _states->get_size();
+  size_t size = _states->get_num_entries();
   for (size_t si = 0; si < size; ++si) {
-    if (!_states->has_element(si)) {
-      continue;
-    }
     const RenderState *state = _states->get_key(si);
 
-    int i;
+    size_t i;
-    int cache_size = state->_composition_cache.get_size();
+    size_t cache_size = state->_composition_cache.get_num_entries();
     for (i = 0; i < cache_size; ++i) {
-      if (state->_composition_cache.has_element(i)) {
       const RenderState *result = state->_composition_cache.get_data(i)._result;
       if (result != (const RenderState *)NULL && result != state) {
         // Here's a RenderState that's recorded in the cache.  Count it.
@@ -805,10 +801,8 @@ get_num_unused_states() {
         }
       }
     }
-    }
+    cache_size = state->_invert_composition_cache.get_num_entries();
-    cache_size = state->_invert_composition_cache.get_size();
     for (i = 0; i < cache_size; ++i) {
-      if (state->_invert_composition_cache.has_element(i)) {
       const RenderState *result = state->_invert_composition_cache.get_data(i)._result;
       if (result != (const RenderState *)NULL && result != state) {
         pair<StateCount::iterator, bool> ir =
@@ -819,7 +813,6 @@ get_num_unused_states() {
       }
     }
   }
-  }
 
   // Now that we have the appearance count of each RenderState object, we can
   // tell which ones are unreferenced outside of the RenderState cache, by
@@ -879,11 +872,8 @@ clear_cache() {
     TempStates temp_states;
     temp_states.reserve(orig_size);
 
-    size_t size = _states->get_size();
+    size_t size = _states->get_num_entries();
     for (size_t si = 0; si < size; ++si) {
-      if (!_states->has_element(si)) {
-        continue;
-      }
       const RenderState *state = _states->get_key(si);
       temp_states.push_back(state);
     }
@@ -894,30 +884,26 @@ clear_cache() {
     for (ti = temp_states.begin(); ti != temp_states.end(); ++ti) {
       RenderState *state = (RenderState *)(*ti).p();
 
-      int i;
+      size_t i;
-      int cache_size = (int)state->_composition_cache.get_size();
+      size_t cache_size = (int)state->_composition_cache.get_num_entries();
       for (i = 0; i < cache_size; ++i) {
-        if (state->_composition_cache.has_element(i)) {
         const RenderState *result = state->_composition_cache.get_data(i)._result;
         if (result != (const RenderState *)NULL && result != state) {
           result->cache_unref();
           nassertr(result->get_ref_count() > 0, 0);
         }
       }
-      }
       _cache_stats.add_total_size(-(int)state->_composition_cache.get_num_entries());
       state->_composition_cache.clear();
 
-      cache_size = (int)state->_invert_composition_cache.get_size();
+      cache_size = (int)state->_invert_composition_cache.get_num_entries();
       for (i = 0; i < cache_size; ++i) {
-        if (state->_invert_composition_cache.has_element(i)) {
         const RenderState *result = state->_invert_composition_cache.get_data(i)._result;
         if (result != (const RenderState *)NULL && result != state) {
           result->cache_unref();
           nassertr(result->get_ref_count() > 0, 0);
         }
       }
-      }
       _cache_stats.add_total_size(-(int)state->_invert_composition_cache.get_num_entries());
       state->_invert_composition_cache.clear();
     }
@@ -950,24 +936,26 @@ garbage_collect() {
   LightReMutexHolder holder(*_states_lock);
 
   PStatTimer timer(_garbage_collect_pcollector);
-  int orig_size = _states->get_num_entries();
+  size_t orig_size = _states->get_num_entries();
 
   // How many elements to process this pass?
-  size_t size = _states->get_size();
+  size_t size = orig_size;
-  size_t num_this_pass = int((int)size * garbage_collect_states_rate);
+  size_t num_this_pass = max(0, int(size * garbage_collect_states_rate));
-  if (size <= 0 || num_this_pass <= 0) {
+  if (num_this_pass <= 0) {
     return num_attribs;
   }
 
   bool break_and_uniquify = (auto_break_cycles && uniquify_transforms);
 
   size_t si = _garbage_index;
+  if (si >= size) {
+    si = 0;
+  }
 
   num_this_pass = min(num_this_pass, size);
-  size_t stop_at_element = (si + num_this_pass) & (size - 1);
+  size_t stop_at_element = (si + num_this_pass) % size;
 
   do {
-    if (_states->has_element(si)) {
     RenderState *state = (RenderState *)_states->get_key(si);
     if (break_and_uniquify) {
       if (state->get_cache_ref_count() > 0 &&
@@ -990,13 +978,20 @@ garbage_collect() {
       state->remove_cache_pointers();
       state->cache_unref();
       delete state;
-      }
+
+      // When we removed it from the hash map, it swapped the last element
+      // with the one we just removed.  So the current index contains one we
+      // still need to visit.
+      --size;
+      --si;
     }
 
-    si = (si + 1) & (size - 1);
+    si = (si + 1) % size;
   } while (si != stop_at_element);
   _garbage_index = si;
 
+  nassertr(_states->get_num_entries() == size, 0);
+
 #ifdef _DEBUG
   nassertr(_states->validate(), 0);
 #endif
@@ -1005,8 +1000,7 @@ garbage_collect() {
   // size.  This will help reduce iteration overhead in the future.
   _states->consider_shrink_table();
 
-  int new_size = _states->get_num_entries();
+  return (int)orig_size - (int)size + num_attribs;
-  return orig_size - new_size + num_attribs;
 }
 
 /**
@@ -1017,11 +1011,8 @@ void RenderState::
 clear_munger_cache() {
   LightReMutexHolder holder(*_states_lock);
 
-  size_t size = _states->get_size();
+  size_t size = _states->get_num_entries();
   for (size_t si = 0; si < size; ++si) {
-    if (!_states->has_element(si)) {
-      continue;
-    }
     RenderState *state = (RenderState *)(_states->get_key(si));
     state->_mungers.clear();
     state->_last_mi = -1;
@@ -1052,11 +1043,8 @@ list_cycles(ostream &out) {
   VisitedStates visited;
   CompositionCycleDesc cycle_desc;
 
-  size_t size = _states->get_size();
+  size_t size = _states->get_num_entries();
   for (size_t si = 0; si < size; ++si) {
-    if (!_states->has_element(si)) {
-      continue;
-    }
     const RenderState *state = _states->get_key(si);
 
     bool inserted = visited.insert(state).second;
@@ -1129,13 +1117,9 @@ list_states(ostream &out) {
   }
   LightReMutexHolder holder(*_states_lock);
 
-  out << _states->get_num_entries() << " states:\n";
+  size_t size = _states->get_num_entries();
-
+  out << size << " states:\n";
-  size_t size = _states->get_size();
   for (size_t si = 0; si < size; ++si) {
-    if (!_states->has_element(si)) {
-      continue;
-    }
     const RenderState *state = _states->get_key(si);
     state->write(out, 2);
   }
@@ -1166,18 +1150,12 @@ validate_states() {
     return false;
   }
 
-  size_t size = _states->get_size();
+  size_t size = _states->get_num_entries();
   size_t si = 0;
-  while (si < size && !_states->has_element(si)) {
-    ++si;
-  }
   nassertr(si < size, false);
   nassertr(_states->get_key(si)->get_ref_count() >= 0, false);
   size_t snext = si;
   ++snext;
-  while (snext < size && !_states->has_element(snext)) {
-    ++snext;
-  }
   while (snext < size) {
     nassertr(_states->get_key(snext)->get_ref_count() >= 0, false);
     const RenderState *ssi = _states->get_key(si);
@@ -1199,9 +1177,6 @@ validate_states() {
     }
     si = snext;
     ++snext;
-    while (snext < size && !_states->has_element(snext)) {
-      ++snext;
-    }
   }
 
   return true;
@@ -1593,10 +1568,9 @@ r_detect_cycles(const RenderState *start_state,
   }
   ((RenderState *)current_state)->_cycle_detect = this_seq;
 
-  int i;
+  size_t i;
-  int cache_size = current_state->_composition_cache.get_size();
+  size_t cache_size = current_state->_composition_cache.get_num_entries();
   for (i = 0; i < cache_size; ++i) {
-    if (current_state->_composition_cache.has_element(i)) {
     const RenderState *result = current_state->_composition_cache.get_data(i)._result;
     if (result != (const RenderState *)NULL) {
       if (r_detect_cycles(start_state, result, length + 1,
@@ -1611,11 +1585,9 @@ r_detect_cycles(const RenderState *start_state,
       }
     }
   }
-  }
 
-  cache_size = current_state->_invert_composition_cache.get_size();
+  cache_size = current_state->_invert_composition_cache.get_num_entries();
   for (i = 0; i < cache_size; ++i) {
-    if (current_state->_invert_composition_cache.has_element(i)) {
     const RenderState *result = current_state->_invert_composition_cache.get_data(i)._result;
     if (result != (const RenderState *)NULL) {
       if (r_detect_cycles(start_state, result, length + 1,
@@ -1630,7 +1602,6 @@ r_detect_cycles(const RenderState *start_state,
       }
     }
   }
-  }
 
   // No cycle detected.
   return false;
@@ -1656,10 +1627,9 @@ r_detect_reverse_cycles(const RenderState *start_state,
   }
   ((RenderState *)current_state)->_cycle_detect = this_seq;
 
-  int i;
+  size_t i;
-  int cache_size = current_state->_composition_cache.get_size();
+  size_t cache_size = current_state->_composition_cache.get_num_entries();
   for (i = 0; i < cache_size; ++i) {
-    if (current_state->_composition_cache.has_element(i)) {
     const RenderState *other = current_state->_composition_cache.get_key(i);
     if (other != current_state) {
       int oi = other->_composition_cache.find(current_state);
@@ -1680,11 +1650,9 @@ r_detect_reverse_cycles(const RenderState *start_state,
       }
     }
   }
-  }
 
-  cache_size = current_state->_invert_composition_cache.get_size();
+  cache_size = current_state->_invert_composition_cache.get_num_entries();
   for (i = 0; i < cache_size; ++i) {
-    if (current_state->_invert_composition_cache.has_element(i)) {
     const RenderState *other = current_state->_invert_composition_cache.get_key(i);
     if (other != current_state) {
       int oi = other->_invert_composition_cache.find(current_state);
@@ -1705,7 +1673,6 @@ r_detect_reverse_cycles(const RenderState *start_state,
       }
     }
   }
-  }
 
   // No cycle detected.
   return false;
@@ -1722,10 +1689,8 @@ release_new() {
   nassertv(_states_lock->debug_is_locked());
 
   if (_saved_entry != -1) {
-    // nassertv(_states->find(this) == _saved_entry);
-    _saved_entry = _states->find(this);
-    _states->remove_element(_saved_entry);
     _saved_entry = -1;
+    nassertv(_states->remove(this));
   }
 }
 
@@ -1771,13 +1736,8 @@ remove_cache_pointers() {
 
   // There are lots of ways to do this loop wrong.  Be very careful if you
   // need to modify it for any reason.
-  int i = 0;
+  size_t i = 0;
   while (!_composition_cache.is_empty()) {
-    // Scan for the next used slot in the table.
-    while (!_composition_cache.has_element(i)) {
-      ++i;
-    }
-
     // It is possible that the "other" RenderState object is currently within
     // its own destructor.  We therefore can't use a PT() to hold its pointer;
     // that could end up calling its destructor twice.  Fortunately, we don't
@@ -1829,10 +1789,6 @@ remove_cache_pointers() {
   // A similar bit of code for the invert cache.
   i = 0;
   while (!_invert_composition_cache.is_empty()) {
-    while (!_invert_composition_cache.has_element(i)) {
-      ++i;
-    }
-
     RenderState *other = (RenderState *)_invert_composition_cache.get_key(i);
     nassertv(other != this);
     Composition comp = _invert_composition_cache.get_data(i);

panda/src/pgraph/renderState_ext.cxx

@@ -30,18 +30,12 @@ PyObject *Extension<RenderState>::
 get_composition_cache() const {
   extern struct Dtool_PyTypedObject Dtool_RenderState;
   LightReMutexHolder holder(*RenderState::_states_lock);
-  size_t cache_size = _this->_composition_cache.get_size();
+  size_t cache_size = _this->_composition_cache.get_num_entries();
   PyObject *list = PyList_New(cache_size);
 
   for (size_t i = 0; i < cache_size; ++i) {
     PyObject *tuple = PyTuple_New(2);
     PyObject *a, *b;
-    if (!_this->_composition_cache.has_element(i)) {
-      a = Py_None;
-      Py_INCREF(a);
-      b = Py_None;
-      Py_INCREF(b);
-    } else {
     const RenderState *source = _this->_composition_cache.get_key(i);
     if (source == (RenderState *)NULL) {
       a = Py_None;
@@ -60,7 +54,6 @@ get_composition_cache() const {
       b = DTool_CreatePyInstanceTyped((void *)result, Dtool_RenderState,
                                       true, true, result->get_type_index());
     }
-    }
     PyTuple_SET_ITEM(tuple, 0, a);
     PyTuple_SET_ITEM(tuple, 1, b);
 
@@ -85,18 +78,12 @@ PyObject *Extension<RenderState>::
 get_invert_composition_cache() const {
   extern struct Dtool_PyTypedObject Dtool_RenderState;
   LightReMutexHolder holder(*RenderState::_states_lock);
-  size_t cache_size = _this->_invert_composition_cache.get_size();
+  size_t cache_size = _this->_invert_composition_cache.get_num_entries();
   PyObject *list = PyList_New(cache_size);
 
   for (size_t i = 0; i < cache_size; ++i) {
     PyObject *tuple = PyTuple_New(2);
     PyObject *a, *b;
-    if (!_this->_invert_composition_cache.has_element(i)) {
-      a = Py_None;
-      Py_INCREF(a);
-      b = Py_None;
-      Py_INCREF(b);
-    } else {
     const RenderState *source = _this->_invert_composition_cache.get_key(i);
     if (source == (RenderState *)NULL) {
       a = Py_None;
@@ -115,7 +102,6 @@ get_invert_composition_cache() const {
       b = DTool_CreatePyInstanceTyped((void *)result, Dtool_RenderState,
                                       true, true, result->get_type_index());
     }
-    }
     PyTuple_SET_ITEM(tuple, 0, a);
     PyTuple_SET_ITEM(tuple, 1, b);
 
@@ -141,11 +127,8 @@ get_states() {
   PyObject *list = PyList_New(num_states);
   size_t i = 0;
 
-  int size = RenderState::_states->get_size();
+  size_t size = RenderState::_states->get_num_entries();
-  for (int si = 0; si < size; ++si) {
+  for (size_t si = 0; si < size; ++si) {
-    if (!RenderState::_states->has_element(si)) {
-      continue;
-    }
     const RenderState *state = RenderState::_states->get_key(si);
     state->ref();
     PyObject *a =

panda/src/pgraph/transformState.I

@@ -664,7 +664,7 @@ get_invert_composition_cache_num_entries() const {
 INLINE size_t TransformState::
 get_composition_cache_size() const {
   LightReMutexHolder holder(*_states_lock);
-  return _composition_cache.get_size();
+  return _composition_cache.get_num_entries();
 }
 
 /**
@@ -678,9 +678,6 @@ get_composition_cache_size() const {
 INLINE const TransformState *TransformState::
 get_composition_cache_source(size_t n) const {
   LightReMutexHolder holder(*_states_lock);
-  if (!_composition_cache.has_element(n)) {
-    return NULL;
-  }
   return _composition_cache.get_key(n);
 }
 
@@ -698,9 +695,6 @@ get_composition_cache_source(size_t n) const {
 INLINE const TransformState *TransformState::
 get_composition_cache_result(size_t n) const {
   LightReMutexHolder holder(*_states_lock);
-  if (!_composition_cache.has_element(n)) {
-    return NULL;
-  }
   return _composition_cache.get_data(n)._result;
 }
 
@@ -716,7 +710,7 @@ get_composition_cache_result(size_t n) const {
 INLINE size_t TransformState::
 get_invert_composition_cache_size() const {
   LightReMutexHolder holder(*_states_lock);
-  return _invert_composition_cache.get_size();
+  return _invert_composition_cache.get_num_entries();
 }
 
 /**
@@ -730,9 +724,6 @@ get_invert_composition_cache_size() const {
 INLINE const TransformState *TransformState::
 get_invert_composition_cache_source(size_t n) const {
   LightReMutexHolder holder(*_states_lock);
-  if (!_invert_composition_cache.has_element(n)) {
-    return NULL;
-  }
   return _invert_composition_cache.get_key(n);
 }
 
@@ -750,9 +741,6 @@ get_invert_composition_cache_source(size_t n) const {
 INLINE const TransformState *TransformState::
 get_invert_composition_cache_result(size_t n) const {
   LightReMutexHolder holder(*_states_lock);
-  if (!_invert_composition_cache.has_element(n)) {
-    return NULL;
-  }
   return _invert_composition_cache.get_data(n)._result;
 }
 

panda/src/pgraph/transformState.cxx

@@ -656,13 +656,13 @@ compose(const TransformState *other) const {
   // The cache entry in this object is the only one that indicates the result;
   // the other will be NULL for now.
   _cache_stats.add_total_size(1);
-  _cache_stats.inc_adds(_composition_cache.get_size() == 0);
+  _cache_stats.inc_adds(_composition_cache.is_empty());
 
   _composition_cache[other]._result = result;
 
   if (other != this) {
     _cache_stats.add_total_size(1);
-    _cache_stats.inc_adds(other->_composition_cache.get_size() == 0);
+    _cache_stats.inc_adds(other->_composition_cache.is_empty());
     other->_composition_cache[this]._result = NULL;
   }
 
@@ -763,12 +763,12 @@ invert_compose(const TransformState *other) const {
   // The cache entry in this object is the only one that indicates the result;
   // the other will be NULL for now.
   _cache_stats.add_total_size(1);
-  _cache_stats.inc_adds(_invert_composition_cache.get_size() == 0);
+  _cache_stats.inc_adds(_invert_composition_cache.is_empty());
   _invert_composition_cache[other]._result = result;
 
   if (other != this) {
     _cache_stats.add_total_size(1);
-    _cache_stats.inc_adds(other->_invert_composition_cache.get_size() == 0);
+    _cache_stats.inc_adds(other->_invert_composition_cache.is_empty());
     other->_invert_composition_cache[this]._result = NULL;
   }
 
@@ -842,11 +842,8 @@ bool TransformState::
 validate_composition_cache() const {
   LightReMutexHolder holder(*_states_lock);
 
-  size_t size = _composition_cache.get_size();
+  size_t size = _composition_cache.get_num_entries();
   for (size_t i = 0; i < size; ++i) {
-    if (!_composition_cache.has_element(i)) {
-      continue;
-    }
     const TransformState *source = _composition_cache.get_key(i);
     if (source != (TransformState *)NULL) {
       // Check that the source also has a pointer back to this one.  We always
@@ -865,11 +862,8 @@ validate_composition_cache() const {
     }
   }
 
-  size = _invert_composition_cache.get_size();
+  size = _invert_composition_cache.get_num_entries();
-  for (int i = 0; i < size; ++i) {
+  for (size_t i = 0; i < size; ++i) {
-    if (!_invert_composition_cache.has_element(i)) {
-      continue;
-    }
     const TransformState *source = _invert_composition_cache.get_key(i);
     if (source != (TransformState *)NULL) {
       // Check that the source also has a pointer back to this one.  We always
@@ -1037,17 +1031,13 @@ get_num_unused_states() {
   typedef pmap<const TransformState *, int> StateCount;
   StateCount state_count;
 
-  size_t size = _states->get_size();
+  size_t size = _states->get_num_entries();
   for (size_t si = 0; si < size; ++si) {
-    if (!_states->has_element(si)) {
-      continue;
-    }
     const TransformState *state = _states->get_key(si);
 
     size_t i;
-    size_t cache_size = state->_composition_cache.get_size();
+    size_t cache_size = state->_composition_cache.get_num_entries();
     for (i = 0; i < cache_size; ++i) {
-      if (state->_composition_cache.has_element(i)) {
       const TransformState *result = state->_composition_cache.get_data(i)._result;
       if (result != (const TransformState *)NULL && result != state) {
         // Here's a TransformState that's recorded in the cache.  Count it.
@@ -1060,10 +1050,8 @@ get_num_unused_states() {
         }
       }
     }
-    }
+    cache_size = state->_invert_composition_cache.get_num_entries();
-    cache_size = state->_invert_composition_cache.get_size();
     for (i = 0; i < cache_size; ++i) {
-      if (state->_invert_composition_cache.has_element(i)) {
       const TransformState *result = state->_invert_composition_cache.get_data(i)._result;
       if (result != (const TransformState *)NULL && result != state) {
         pair<StateCount::iterator, bool> ir =
@@ -1074,7 +1062,6 @@ get_num_unused_states() {
       }
     }
   }
-  }
 
   // Now that we have the appearance count of each TransformState object, we
   // can tell which ones are unreferenced outside of the TransformState cache,
@@ -1135,11 +1122,8 @@ clear_cache() {
     TempStates temp_states;
     temp_states.reserve(orig_size);
 
-    size_t size = _states->get_size();
+    size_t size = _states->get_num_entries();
     for (size_t si = 0; si < size; ++si) {
-      if (!_states->has_element(si)) {
-        continue;
-      }
       const TransformState *state = _states->get_key(si);
       temp_states.push_back(state);
     }
@@ -1150,30 +1134,26 @@ clear_cache() {
     for (ti = temp_states.begin(); ti != temp_states.end(); ++ti) {
       TransformState *state = (TransformState *)(*ti).p();
 
-      int i;
+      size_t i;
-      int cache_size = (int)state->_composition_cache.get_size();
+      size_t cache_size = state->_composition_cache.get_num_entries();
       for (i = 0; i < cache_size; ++i) {
-        if (state->_composition_cache.has_element(i)) {
         const TransformState *result = state->_composition_cache.get_data(i)._result;
         if (result != (const TransformState *)NULL && result != state) {
           result->cache_unref();
           nassertr(result->get_ref_count() > 0, 0);
         }
       }
-      }
       _cache_stats.add_total_size(-(int)state->_composition_cache.get_num_entries());
       state->_composition_cache.clear();
 
-      cache_size = state->_invert_composition_cache.get_size();
+      cache_size = state->_invert_composition_cache.get_num_entries();
       for (i = 0; i < cache_size; ++i) {
-        if (state->_invert_composition_cache.has_element(i)) {
         const TransformState *result = state->_invert_composition_cache.get_data(i)._result;
         if (result != (const TransformState *)NULL && result != state) {
           result->cache_unref();
           nassertr(result->get_ref_count() > 0, 0);
         }
       }
-      }
       _cache_stats.add_total_size(-(int)state->_invert_composition_cache.get_num_entries());
       state->_invert_composition_cache.clear();
     }
@@ -1199,27 +1179,29 @@ garbage_collect() {
     return 0;
   }
 
-  bool break_and_uniquify = (auto_break_cycles && uniquify_transforms);
-
   LightReMutexHolder holder(*_states_lock);
 
   PStatTimer timer(_garbage_collect_pcollector);
   size_t orig_size = _states->get_num_entries();
 
   // How many elements to process this pass?
-  size_t size = _states->get_size();
+  size_t size = orig_size;
-  size_t num_this_pass = int(size * garbage_collect_states_rate);
+  size_t num_this_pass = max(0, int(size * garbage_collect_states_rate));
-  if (size <= 0 || num_this_pass <= 0) {
+  if (num_this_pass <= 0) {
     return 0;
   }
 
+  bool break_and_uniquify = (auto_break_cycles && uniquify_transforms);
+
   size_t si = _garbage_index;
+  if (si >= size) {
+    si = 0;
+  }
 
   num_this_pass = min(num_this_pass, size);
-  size_t stop_at_element = (si + num_this_pass) & (size - 1);
+  size_t stop_at_element = (si + num_this_pass) % size;
 
   do {
-    if (_states->has_element(si)) {
     TransformState *state = (TransformState *)_states->get_key(si);
     if (break_and_uniquify) {
       if (state->get_cache_ref_count() > 0 &&
@@ -1242,13 +1224,20 @@ garbage_collect() {
       state->remove_cache_pointers();
       state->cache_unref();
       delete state;
-      }
+
+      // When we removed it from the hash map, it swapped the last element
+      // with the one we just removed.  So the current index contains one we
+      // still need to visit.
+      --size;
+      --si;
     }
 
-    si = (si + 1) & (size - 1);
+    si = (si + 1) % size;
   } while (si != stop_at_element);
   _garbage_index = si;
 
+  nassertr(_states->get_num_entries() == size, 0);
+
 #ifdef _DEBUG
   nassertr(_states->validate(), 0);
 #endif
@@ -1257,8 +1246,7 @@ garbage_collect() {
   // size.  This will help reduce iteration overhead in the future.
   _states->consider_shrink_table();
 
-  int new_size = _states->get_num_entries();
+  return (int)orig_size - (int)size;
-  return orig_size - new_size;
 }
 
 /**
@@ -1285,11 +1273,8 @@ list_cycles(ostream &out) {
   VisitedStates visited;
   CompositionCycleDesc cycle_desc;
 
-  size_t size = _states->get_size();
+  size_t size = _states->get_num_entries();
   for (size_t si = 0; si < size; ++si) {
-    if (!_states->has_element(si)) {
-      continue;
-    }
     const TransformState *state = _states->get_key(si);
 
     bool inserted = visited.insert(state).second;
@@ -1362,13 +1347,9 @@ list_states(ostream &out) {
   }
   LightReMutexHolder holder(*_states_lock);
 
-  out << _states->get_num_entries() << " states:\n";
+  size_t size = _states->get_num_entries();
-
+  out << size << " states:\n";
-  size_t size = _states->get_size();
   for (size_t si = 0; si < size; ++si) {
-    if (!_states->has_element(si)) {
-      continue;
-    }
     const TransformState *state = _states->get_key(si);
     state->write(out, 2);
   }
@@ -1399,18 +1380,12 @@ validate_states() {
     return false;
   }
 
-  size_t size = _states->get_size();
+  size_t size = _states->get_num_entries();
   size_t si = 0;
-  while (si < size && !_states->has_element(si)) {
-    ++si;
-  }
   nassertr(si < size, false);
   nassertr(_states->get_key(si)->get_ref_count() >= 0, false);
   size_t snext = si;
   ++snext;
-  while (snext < size && !_states->has_element(snext)) {
-    ++snext;
-  }
   while (snext < size) {
     nassertr(_states->get_key(snext)->get_ref_count() >= 0, false);
     const TransformState *ssi = _states->get_key(si);
@@ -1433,9 +1408,6 @@ validate_states() {
     }
     si = snext;
     ++snext;
-    while (snext < size && !_states->has_element(snext)) {
-      ++snext;
-    }
   }
 
   return true;
@@ -1805,10 +1777,9 @@ r_detect_cycles(const TransformState *start_state,
   }
   ((TransformState *)current_state)->_cycle_detect = this_seq;
 
-  int i;
+  size_t i;
-  int cache_size = current_state->_composition_cache.get_size();
+  size_t cache_size = current_state->_composition_cache.get_num_entries();
   for (i = 0; i < cache_size; ++i) {
-    if (current_state->_composition_cache.has_element(i)) {
     const TransformState *result = current_state->_composition_cache.get_data(i)._result;
     if (result != (const TransformState *)NULL) {
       if (r_detect_cycles(start_state, result, length + 1,
@@ -1823,11 +1794,9 @@ r_detect_cycles(const TransformState *start_state,
       }
     }
   }
-  }
 
-  cache_size = current_state->_invert_composition_cache.get_size();
+  cache_size = current_state->_invert_composition_cache.get_num_entries();
   for (i = 0; i < cache_size; ++i) {
-    if (current_state->_invert_composition_cache.has_element(i)) {
     const TransformState *result = current_state->_invert_composition_cache.get_data(i)._result;
     if (result != (const TransformState *)NULL) {
       if (r_detect_cycles(start_state, result, length + 1,
@@ -1842,7 +1811,6 @@ r_detect_cycles(const TransformState *start_state,
       }
     }
   }
-  }
 
   // No cycle detected.
   return false;
@@ -1868,10 +1836,9 @@ r_detect_reverse_cycles(const TransformState *start_state,
   }
   ((TransformState *)current_state)->_cycle_detect = this_seq;
 
-  int i;
+  size_t i;
-  int cache_size = current_state->_composition_cache.get_size();
+  size_t cache_size = current_state->_composition_cache.get_num_entries();
   for (i = 0; i < cache_size; ++i) {
-    if (current_state->_composition_cache.has_element(i)) {
     const TransformState *other = current_state->_composition_cache.get_key(i);
     if (other != current_state) {
       int oi = other->_composition_cache.find(current_state);
@@ -1892,11 +1859,9 @@ r_detect_reverse_cycles(const TransformState *start_state,
       }
     }
   }
-  }
 
-  cache_size = current_state->_invert_composition_cache.get_size();
+  cache_size = current_state->_invert_composition_cache.get_num_entries();
   for (i = 0; i < cache_size; ++i) {
-    if (current_state->_invert_composition_cache.has_element(i)) {
     const TransformState *other = current_state->_invert_composition_cache.get_key(i);
     if (other != current_state) {
       int oi = other->_invert_composition_cache.find(current_state);
@@ -1917,7 +1882,6 @@ r_detect_reverse_cycles(const TransformState *start_state,
       }
     }
   }
-  }
 
   // No cycle detected.
   return false;
@@ -1935,10 +1899,8 @@ release_new() {
   nassertv(_states_lock->debug_is_locked());
 
   if (_saved_entry != -1) {
-    // nassertv(_states->find(this) == _saved_entry);
-    _saved_entry = _states->find(this);
-    _states->remove_element(_saved_entry);
     _saved_entry = -1;
+    nassertv(_states->remove(this));
   }
 }
 
@@ -1977,13 +1939,8 @@ remove_cache_pointers() {
 
   // There are lots of ways to do this loop wrong.  Be very careful if you
   // need to modify it for any reason.
-  int i = 0;
+  size_t i = 0;
   while (!_composition_cache.is_empty()) {
-    // Scan for the next used slot in the table.
-    while (!_composition_cache.has_element(i)) {
-      ++i;
-    }
-
     // It is possible that the "other" TransformState object is currently
     // within its own destructor.  We therefore can't use a PT() to hold its
     // pointer; that could end up calling its destructor twice.  Fortunately,
@@ -2036,10 +1993,6 @@ remove_cache_pointers() {
   // A similar bit of code for the invert cache.
   i = 0;
   while (!_invert_composition_cache.is_empty()) {
-    while (!_invert_composition_cache.has_element(i)) {
-      ++i;
-    }
-
     TransformState *other = (TransformState *)_invert_composition_cache.get_key(i);
     nassertv(other != this);
     Composition comp = _invert_composition_cache.get_data(i);

panda/src/pgraph/transformState_ext.cxx

@@ -35,12 +35,8 @@ get_composition_cache() const {
   PyObject *list = PyList_New(num_states);
   size_t i = 0;
 
-  int size = _this->_composition_cache.get_size();
+  size_t size = _this->_composition_cache.get_num_entries();
-  for (int si = 0; si < size; ++si) {
+  for (size_t si = 0; si < size; ++si) {
-    if (!_this->_composition_cache.has_element(si)) {
-      continue;
-    }
-
     PyObject *tuple = PyTuple_New(2);
     PyObject *a, *b;
 
@@ -94,12 +90,8 @@ get_invert_composition_cache() const {
   PyObject *list = PyList_New(num_states);
   size_t i = 0;
 
-  int size = _this->_invert_composition_cache.get_size();
+  size_t size = _this->_invert_composition_cache.get_num_entries();
-  for (int si = 0; si < size; ++si) {
+  for (size_t si = 0; si < size; ++si) {
-    if (!_this->_invert_composition_cache.has_element(si)) {
-      continue;
-    }
-
     PyObject *tuple = PyTuple_New(2);
     PyObject *a, *b;
 
@@ -149,11 +141,8 @@ get_states() {
   PyObject *list = PyList_New(num_states);
   size_t i = 0;
 
-  int size = TransformState::_states->get_size();
+  size_t size = TransformState::_states->get_num_entries();
-  for (int si = 0; si < size; ++si) {
+  for (size_t si = 0; si < size; ++si) {
-    if (!TransformState::_states->has_element(si)) {
-      continue;
-    }
     const TransformState *state = TransformState::_states->get_key(si);
     state->ref();
     PyObject *a =
@@ -180,11 +169,8 @@ get_unused_states() {
   LightReMutexHolder holder(*TransformState::_states_lock);
 
   PyObject *list = PyList_New(0);
-  int size = TransformState::_states->get_size();
+  size_t size = TransformState::_states->get_num_entries();
-  for (int si = 0; si < size; ++si) {
+  for (size_t si = 0; si < size; ++si) {
-    if (!TransformState::_states->has_element(si)) {
-      continue;
-    }
     const TransformState *state = TransformState::_states->get_key(si);
     if (state->get_cache_ref_count() == state->get_ref_count()) {
       state->ref();

panda/src/putil/simpleHashMap.I

@@ -15,16 +15,31 @@
  *
  */
 template<class Key, class Value, class Compare>
-INLINE SimpleHashMap<Key, Value, Compare>::
+CONSTEXPR SimpleHashMap<Key, Value, Compare>::
 SimpleHashMap(const Compare &comp) :
-  _table(NULL),
+  _table(nullptr),
-  _deleted_chain(NULL),
+  _deleted_chain(nullptr),
   _table_size(0),
   _num_entries(0),
   _comp(comp)
 {
 }
 
+/**
+ *
+ */
+template<class Key, class Value, class Compare>
+INLINE SimpleHashMap<Key, Value, Compare>::
+SimpleHashMap(SimpleHashMap &&from) NOEXCEPT :
+  _table(from._table),
+  _deleted_chain(from._deleted_chain),
+  _table_size(from._table_size),
+  _num_entries(from._num_entries),
+  _comp(move(from._comp))
+{
+  from._table_size = 0;
+}
+
 /**
  *
  */
@@ -69,30 +84,14 @@ find(const Key &key) const {
     return -1;
   }
 
-  size_t index = get_hash(key);
+  int slot = find_slot(key);
-  if (!has_element(index)) {
+  if (slot >= 0) {
-    return -1;
+    return get_index_array()[slot];
-  }
+  } else {
-  if (is_element(index, key)) {
-    return index;
-  }
-
-  // There was some other key at the hashed slot.  That's a hash conflict.
-  // Maybe our entry was recorded at a later slot position; scan the
-  // subsequent positions until we find the entry or an unused slot,
-  // indicating the end of the scan.
-  size_t i = index;
-  i = (i + 1) & (_table_size - 1);
-  while (i != index && has_element(i)) {
-    if (is_element(i, key)) {
-      return i;
-    }
-    i = (i + 1) & (_table_size - 1);
-  }
-
     // The key is not in the table.
     return -1;
   }
+}
 
 /**
  * Records the indicated key/data pair in the map.  If the key was already
@@ -105,23 +104,23 @@ store(const Key &key, const Value &data) {
     // Special case: the first key in an empty table.
     nassertr(_num_entries == 0, -1);
     new_table();
-    size_t index = get_hash(key);
+    int pos = store_new_element(get_hash(key), key, data);
-    store_new_element(index, key, data);
-    ++_num_entries;
 #ifdef _DEBUG
-    nassertr(validate(), index);
+    nassertr(validate(), pos);
 #endif
-    return index;
+    return pos;
   }
+  consider_expand_table();
 
-  size_t index = get_hash(key);
+  const int *index_array = get_index_array();
-  if (!has_element(index)) {
+  size_t hash = get_hash(key);
+  int index = index_array[hash];
+  if (index < 0) {
     // This element is not already in the map; add it.
     if (consider_expand_table()) {
       return store(key, data);
     }
-    store_new_element(index, key, data);
+    index = store_new_element(hash, key, data);
-    ++_num_entries;
 #ifdef _DEBUG
     nassertr(validate(), index);
 #endif
@@ -138,28 +137,27 @@ store(const Key &key, const Value &data) {
 
   // There was some other key at the hashed slot.  That's a hash conflict.
   // Record this entry at a later position.
-  size_t i = index;
+  size_t slot = next_hash(hash);
-  i = (i + 1) & (_table_size - 1);
+  while (slot != hash) {
-  while (i != index) {
+    index = index_array[slot];
-    if (!has_element(i)) {
+    if (index < 0) {
       if (consider_expand_table()) {
         return store(key, data);
       }
-      store_new_element(i, key, data);
+      index = store_new_element(slot, key, data);
-      ++_num_entries;
 #ifdef _DEBUG
-      nassertr(validate(), i);
+      nassertr(validate(), index);
 #endif
-      return i;
+      return index;
     }
-    if (is_element(i, key)) {
+    if (is_element(index, key)) {
-      _table[i]._data = data;
+      _table[index]._data = data;
 #ifdef _DEBUG
-      nassertr(validate(), i);
+      nassertr(validate(), index);
 #endif
-      return i;
+      return index;
     }
-    i = (i + 1) & (_table_size - 1);
+    slot = next_hash(slot);
   }
 
   // Shouldn't get here unless _num_entries == _table_size, which shouldn't be
@@ -171,15 +169,83 @@ store(const Key &key, const Value &data) {
 /**
  * Removes the indicated key and its associated data from the table.  Returns
  * true if the key was removed, false if it was not present.
+ *
+ * Iterator safety:  To perform removal during iteration, revisit the element
+ * at the current index if removal succeeds,  keeping in mind that the number
+ * of elements has now shrunk by one.
  */
 template<class Key, class Value, class Compare>
 INLINE bool SimpleHashMap<Key, Value, Compare>::
 remove(const Key &key) {
-  int index = find(key);
+  if (_num_entries == 0) {
-  if (index == -1) {
+    // Special case: the table is empty.
     return false;
   }
-  remove_element(index);
+
+  int *index_array = get_index_array();
+  size_t slot = (size_t)find_slot(key);
+  if (slot == (size_t)-1) {
+    // It wasn't in the hash map.
+    return false;
+  }
+
+  // Now remove this element.
+  size_t last = _num_entries - 1;
+  size_t index = (size_t)index_array[slot];
+  if (index < _num_entries) {
+    // Find the last element in the index array.
+    int other_slot = find_slot(_table[last]._key);
+    nassertr(other_slot != -1, false);
+    nassertr(index_array[(size_t)other_slot] == (int)last, false);
+
+    // 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);
+    index_array[(size_t)other_slot] = index;
+  }
+
+  _table[last].~TableEntry();
+  _num_entries = last;
+
+  // It's important that we do this after the second find_slot, above, since
+  // it might otherwise fail due to the unexpected gap, since some indices may
+  // not be at their ideal positions right now.
+  index_array[slot] = -1;
+
+  //if (consider_shrink_table()) {
+  //  // No need to worry about that gap; resize_table() will rebuild the index.
+  //  return true;
+  //}
+
+  // Now we have put a hole in the index array.  If there was a hash conflict
+  // in the slot after this one, we have to move it down to close the hole.
+  slot = next_hash(slot);
+  while (has_slot(slot)) {
+    size_t index = (size_t)index_array[slot];
+    size_t wants_slot = get_hash(_table[index]._key);
+    if (wants_slot != slot) {
+      // This one was a hash conflict; try to put it where it belongs.  We
+      // can't just put it in n, since maybe it belongs somewhere after n.
+      while (wants_slot != slot && has_slot(wants_slot)) {
+        wants_slot = next_hash(wants_slot);
+      }
+      if (wants_slot != slot) {
+        // We just have to flip the slots in the index array; we can keep the
+        // elements in the table where they are.
+        index_array[wants_slot] = index;
+        index_array[slot] = -1;
+      }
+    }
+
+    // Continue until we encounter the next unused slot.  Until we do, we
+    // can't be sure we've found all of the potential hash conflicts.
+    slot = next_hash(slot);
+  }
+
+#ifdef _DEBUG
+  nassertr(validate(), true);
+#endif
   return true;
 }
 
@@ -190,15 +256,13 @@ template<class Key, class Value, class Compare>
 void SimpleHashMap<Key, Value, Compare>::
 clear() {
   if (_table_size != 0) {
-    for (size_t i = 0; i < _table_size; ++i) {
+    for (size_t i = 0; i < _num_entries; ++i) {
-      if (has_element(i)) {
+      _table[i].~TableEntry();
-        clear_element(i);
-      }
     }
 
     _deleted_chain->deallocate(_table, TypeHandle::none());
-    _table = NULL;
+    _table = nullptr;
-    _deleted_chain = NULL;
+    _deleted_chain = nullptr;
     _table_size = 0;
     _num_entries = 0;
   }
@@ -219,131 +283,76 @@ operator [] (const Key &key) {
 }
 
 /**
- * Returns the total number of slots in the table.
+ * Returns the total number of entries in the table.  Same as get_num_entries.
  */
 template<class Key, class Value, class Compare>
-INLINE size_t SimpleHashMap<Key, Value, Compare>::
+CONSTEXPR size_t SimpleHashMap<Key, Value, Compare>::
-get_size() const {
+size() const {
-  return _table_size;
+  return _num_entries;
 }
 
 /**
- * Returns true if there is an element stored in the nth slot, false
+ * Returns the key in the nth entry of the table.
- * otherwise.
  *
- * n should be in the range 0 <= n < get_size().
+ * @param n should be in the range 0 <= n < size().
- */
-template<class Key, class Value, class Compare>
-INLINE bool SimpleHashMap<Key, Value, Compare>::
-has_element(size_t n) const {
-  nassertr(n < _table_size, false);
-  return (get_exists_array()[n] != 0);
-}
-
-/**
- * Returns the key in the nth slot of the table.
- *
- * It is an error to call this if there is nothing stored in the nth slot (use
- * has_element() to check this first).  n should be in the range 0 <= n <
- * get_size().
  */
 template<class Key, class Value, class Compare>
 INLINE const Key &SimpleHashMap<Key, Value, Compare>::
 get_key(size_t n) const {
-  nassertr(has_element(n), _table[n]._key);
+  nassertr(n < _num_entries, _table[n]._key);
   return _table[n]._key;
 }
 
 /**
- * Returns the data in the nth slot of the table.
+ * Returns the data in the nth entry of the table.
  *
- * It is an error to call this if there is nothing stored in the nth slot (use
+ * @param n should be in the range 0 <= n < size().
- * has_element() to check this first).  n should be in the range 0 <= n <
- * get_size().
  */
 template<class Key, class Value, class Compare>
 INLINE const Value &SimpleHashMap<Key, Value, Compare>::
 get_data(size_t n) const {
-  nassertr(has_element(n), _table[n]._data);
+  nassertr(n < _num_entries, _table[n]._data);
   return _table[n]._data;
 }
 
 /**
- * Returns a modifiable reference to the data in the nth slot of the table.
+ * Returns a modifiable reference to the data in the nth entry of the table.
  *
- * It is an error to call this if there is nothing stored in the nth slot (use
+ * @param n should be in the range 0 <= n < size().
- * has_element() to check this first).  n should be in the range 0 <= n <
- * get_size().
  */
 template<class Key, class Value, class Compare>
 INLINE Value &SimpleHashMap<Key, Value, Compare>::
 modify_data(size_t n) {
-  nassertr(has_element(n), _table[n]._data);
+  nassertr(n < _num_entries, _table[n]._data);
   return _table[n]._data;
 }
 
 /**
- * Changes the data for the nth slot of the table.
+ * Changes the data for the nth entry of the table.
  *
- * It is an error to call this if there is nothing stored in the nth slot (use
+ * @param n should be in the range 0 <= n < size().
- * has_element() to check this first).  n should be in the range 0 <= n <
- * get_size().
  */
 template<class Key, class Value, class Compare>
 INLINE void SimpleHashMap<Key, Value, Compare>::
 set_data(size_t n, const Value &data) {
-  nassertv(has_element(n));
+  nassertv(n < _num_entries);
   _table[n]._data = data;
 }
 
 /**
- * Removes the nth slot from the table.
+ * Removes the nth entry from the table.
  *
- * It is an error to call this if there is nothing stored in the nth slot (use
+ * @param n should be in the range 0 <= n < size().
- * has_element() to check this first).  n should be in the range 0 <= n <
- * get_size().
  */
 template<class Key, class Value, class Compare>
 void SimpleHashMap<Key, Value, Compare>::
 remove_element(size_t n) {
-  nassertv(has_element(n));
+  nassertv(n < _num_entries);
-
+  remove(_table[n]._key);
-  clear_element(n);
-  nassertv(_num_entries > 0);
-  --_num_entries;
-
-  // Now we have put a hole in the table.  If there was a hash conflict in the
-  // slot following this one, we have to move it down to close the hole.
-  size_t i = n;
-  i = (i + 1) & (_table_size - 1);
-  while (has_element(i)) {
-    size_t wants_index = get_hash(_table[i]._key);
-    if (wants_index != i) {
-      // This one was a hash conflict; try to put it where it belongs.  We
-      // can't just put it in n, since maybe it belongs somewhere after n.
-      while (wants_index != i && has_element(wants_index)) {
-        wants_index = (wants_index + 1) & (_table_size - 1);
-      }
-      if (wants_index != i) {
-        store_new_element(wants_index, _table[i]._key, _table[i]._data);
-        clear_element(i);
-      }
-    }
-
-    // Continue until we encounter the next unused slot.  Until we do, we
-    // can't be sure we've found all of the potential hash conflicts.
-    i = (i + 1) & (_table_size - 1);
-  }
-
-#ifdef _DEBUG
-  nassertv(validate());
-#endif
 }
 
 /**
- * Returns the number of active entries in the table.  This is not necessarily
+ * Returns the number of active entries in the table.  Same as size().
- * related to the number of slots in the table as reported by get_size().  Use
- * get_size() to iterate through all of the slots, not get_num_entries().
  */
 template<class Key, class Value, class Compare>
 INLINE size_t SimpleHashMap<Key, Value, Compare>::
@@ -367,17 +376,20 @@ template<class Key, class Value, class Compare>
 void SimpleHashMap<Key, Value, Compare>::
 output(ostream &out) const {
   out << "SimpleHashMap (" << _num_entries << " entries): [";
-  for (size_t i = 0; i < _table_size; ++i) {
+  const int *index_array = get_index_array();
-    if (!has_element(i)) {
+  size_t num_slots = _table_size * sparsity;
+  for (size_t slot = 0; slot < num_slots; ++slot) {
+    if (!has_slot(slot)) {
       out << " *";
 
     } else {
-      out << " " << _table[i]._key;
+      size_t index = (size_t)index_array[slot];
-      size_t index = get_hash(_table[i]._key);
+      out << " " << index;
-      if (index != i) {
+      size_t ideal_slot = get_hash(_table[index]._key);
+      if (ideal_slot != slot) {
         // This was misplaced as the result of a hash conflict.  Report how
         // far off it is.
-        out << "(" << ((_table_size + i - index) & (_table_size - 1)) << ")";
+        out << "(" << ((_table_size + slot - ideal_slot) & (num_slots - 1)) << ")";
       }
     }
   }
@@ -392,6 +404,9 @@ void SimpleHashMap<Key, Value, Compare>::
 write(ostream &out) const {
   output(out);
   out << "\n";
+  for (size_t i = 0; i < _num_entries; ++i) {
+    out << "  " << _table[i]._key << " (hash " << get_hash(_table[i]._key) << ")\n";
+  }
 }
 
 /**
@@ -403,21 +418,31 @@ bool SimpleHashMap<Key, Value, Compare>::
 validate() const {
   size_t count = 0;
 
-  const unsigned char *exists_array = get_exists_array();
+  const int *index_array = get_index_array();
-
+  size_t num_slots = _table_size * sparsity;
-  for (size_t i = 0; i < _table_size; ++i) {
+  for (size_t slot = 0; slot < num_slots; ++slot) {
-    if (exists_array[i] != 0) {
+    if (has_slot(slot)) {
+      size_t index = (size_t)index_array[slot];
       ++count;
-      size_t ideal_index = get_hash(_table[i]._key);
+      if (index >= _num_entries) {
-      size_t wants_index = ideal_index;
-      while (wants_index != i && exists_array[wants_index] != 0) {
-        wants_index = (wants_index + 1) & (_table_size - 1);
-      }
-      if (wants_index != i) {
         util_cat.error()
-          << "SimpleHashMap is invalid: key " << _table[i]._key
+          << "SimpleHashMap " << this << " is invalid: slot " << slot
-          << " should be in slot " << wants_index << " instead of "
+          << " contains index " << index << " which is past the end of the"
-          << i << " (ideal is " << ideal_index << ")\n";
+             " table\n";
+        write(util_cat.error(false));
+        return false;
+      }
+      nassertd(index < _num_entries) continue;
+      size_t ideal_slot = get_hash(_table[index]._key);
+      size_t wants_slot = ideal_slot;
+      while (wants_slot != slot && has_slot(wants_slot)) {
+        wants_slot = next_hash(wants_slot);
+      }
+      if (wants_slot != slot) {
+        util_cat.error()
+          << "SimpleHashMap " << this << " is invalid: key "
+          << _table[index]._key << " should be in slot " << wants_slot
+          << " instead of " << slot << " (ideal is " << ideal_slot << ")\n";
         write(util_cat.error(false));
         return false;
       }
@@ -426,7 +451,7 @@ validate() const {
 
   if (count != _num_entries) {
     util_cat.error()
-      << "SimpleHashMap is invalid: reports " << _num_entries
+      << "SimpleHashMap " << this << " is invalid: reports " << _num_entries
       << " entries, actually has " << count << "\n";
     write(util_cat.error(false));
     return false;
@@ -449,7 +474,57 @@ get_hash(const Key &key) const {
   return (size_t)floor(f * _table_size);
   */
 
-  return ((_comp(key) * (size_t)9973) >> 8) & (_table_size - 1);
+  return ((_comp(key) * (size_t)9973) >> 8) & ((_table_size * sparsity) - 1);
+}
+
+/**
+ * Given a hash value, increments it, looping around the hash space.
+ */
+template<class Key, class Value, class Compare>
+INLINE size_t SimpleHashMap<Key, Value, Compare>::
+next_hash(size_t hash) const {
+  return (hash + 1) & ((_table_size * sparsity) - 1);
+}
+
+/**
+ * Finds the slot in which the given key should fit.
+ */
+template<class Key, class Value, class Compare>
+INLINE int SimpleHashMap<Key, Value, Compare>::
+find_slot(const Key &key) const {
+  const int *index_array = get_index_array();
+  size_t hash = get_hash(key);
+  int index = index_array[hash];
+  if (index < 0) {
+    return -1;
+  }
+
+  if (is_element((size_t)index, key)) {
+    return hash;
+  }
+
+  // There was some other key at the hashed slot.  That's a hash conflict.
+  // Maybe our entry was recorded at a later slot position; scan the
+  // subsequent positions until we find the entry or an unused slot,
+  // indicating the end of the scan.
+  size_t slot = next_hash(hash);
+  while (slot != hash && has_slot(slot)) {
+    if (is_element((size_t)index_array[slot], key)) {
+      return (int)slot;
+    }
+    slot = next_hash(slot);
+  }
+
+  return -1;
+}
+
+/**
+ * Returns true if the given slot refers to an element.
+ */
+template<class Key, class Value, class Compare>
+INLINE bool SimpleHashMap<Key, Value, Compare>::
+has_slot(size_t slot) const {
+  return get_index_array()[slot] >= 0;
 }
 
 /**
@@ -458,40 +533,33 @@ get_hash(const Key &key) const {
 template<class Key, class Value, class Compare>
 INLINE bool SimpleHashMap<Key, Value, Compare>::
 is_element(size_t n, const Key &key) const {
-  nassertr(has_element(n), false);
+  nassertr(n < _num_entries, false);
   return _comp.is_equal(_table[n]._key, key);
 }
 
 /**
- * Constructs a new TableEntry at position n, storing the indicated key and
+ * Constructs a new TableEntry with the given slot, storing the indicated key
- * value.
+ * and value.
  */
 template<class Key, class Value, class Compare>
-INLINE void SimpleHashMap<Key, Value, Compare>::
+INLINE size_t SimpleHashMap<Key, Value, Compare>::
-store_new_element(size_t n, const Key &key, const Value &data) {
+store_new_element(size_t slot, const Key &key, const Value &data) {
-  new(&_table[n]) TableEntry(key, data);
+  size_t index = _num_entries++;
-  get_exists_array()[n] = true;
+  new(&_table[index]) TableEntry(key, data);
+  nassertr(get_index_array()[slot] == -1, index)
+  get_index_array()[slot] = index;
+  return index;
 }
 
 /**
- * Destructs the TableEntry at position n.
+ * Returns the beginning of the array of _table_size ints that are the indices
- */
+ * pointing to the location within the table where the elements are stored.
-template<class Key, class Value, class Compare>
-INLINE void SimpleHashMap<Key, Value, Compare>::
-clear_element(size_t n) {
-  _table[n].~TableEntry();
-  get_exists_array()[n] = false;
-}
-
-/**
- * Returns the beginning of the array of _table_size unsigned chars that are
- * the boolean flags for whether each element exists (has been constructed)
  * within the table.
  */
 template<class Key, class Value, class Compare>
-INLINE unsigned char *SimpleHashMap<Key, Value, Compare>::
+INLINE int *SimpleHashMap<Key, Value, Compare>::
-get_exists_array() const {
+get_index_array() const {
-  return (unsigned char *)(_table + _table_size);
+  return (int *)(_table + _table_size);
 }
 
 /**
@@ -504,15 +572,15 @@ new_table() {
 
   // Pick a good initial table size.  For now, we make it really small.  Maybe
   // that's the right answer.
-  _table_size = 4;
+  _table_size = 2;
 
   // We allocate enough bytes for _table_size elements of TableEntry, plus
-  // _table_size more bytes at the end (for the exists array).
+  // _table_size * 4 more ints at the end (for the index array).
-  size_t alloc_size = _table_size * sizeof(TableEntry) + _table_size;
+  size_t alloc_size = _table_size * (sizeof(TableEntry) + sizeof(int) * sparsity);
 
   _deleted_chain = memory_hook->get_deleted_chain(alloc_size);
   _table = (TableEntry *)_deleted_chain->allocate(alloc_size, TypeHandle::none());
-  memset(get_exists_array(), 0, _table_size);
+  memset(get_index_array(), -1, _table_size * sizeof(int) * sparsity);
 }
 
 /**
@@ -522,7 +590,7 @@ new_table() {
 template<class Key, class Value, class Compare>
 INLINE bool SimpleHashMap<Key, Value, Compare>::
 consider_expand_table() {
-  if (_num_entries < (_table_size >> 1)) {
+  if (_num_entries < _table_size) {
     return false;
   } else {
     resize_table(_table_size << 1);
@@ -538,14 +606,14 @@ template<class Key, class Value, class Compare>
 INLINE bool SimpleHashMap<Key, Value, Compare>::
 consider_shrink_table() {
   // If the number of elements gets less than an eighth of the table size, we
-  // know it's probably time to shrink it down, lest it hurt iteration time.
+  // know it's probably time to shrink it down.
-  if (_table_size <= 8 || (_table_size >> 3) < _num_entries) {
+  if (_table_size <= 16 || _num_entries >= (_table_size >> 3)) {
     return false;
   } else {
     size_t new_size = _table_size;
     do {
       new_size >>= 1;
-    } while ((new_size >> 3) >= _num_entries);
+    } while (new_size >= 16 && _num_entries < (new_size >> 2));
     resize_table(new_size);
     return true;
   }
@@ -558,47 +626,42 @@ template<class Key, class Value, class Compare>
 void SimpleHashMap<Key, Value, Compare>::
 resize_table(size_t new_size) {
   nassertv(_table_size != 0);
+  nassertv(new_size >= _num_entries);
 
-  SimpleHashMap<Key, Value, Compare> old_map(_comp);
+  DeletedBufferChain *old_chain = _deleted_chain;
-  swap(old_map);
+  TableEntry *old_table = _table;
 
-  size_t old_table_size = old_map._table_size;
   _table_size = new_size;
-  nassertv(_table == NULL);
 
   // We allocate enough bytes for _table_size elements of TableEntry, plus
-  // _table_size more bytes at the end (for the exists array).
+  // _table_size * sparsity more ints at the end (for the sparse index array).
-  size_t alloc_size = _table_size * sizeof(TableEntry) + _table_size;
+  size_t alloc_size = _table_size * sizeof(TableEntry) + _table_size * sparsity * sizeof(int);
   _deleted_chain = memory_hook->get_deleted_chain(alloc_size);
   _table = (TableEntry *)_deleted_chain->allocate(alloc_size, TypeHandle::none());
-  unsigned char *exists_array = get_exists_array();
+  int *index_array = get_index_array();
-  memset(exists_array, 0, _table_size);
+  memset(index_array, -1, _table_size * sizeof(int) * sparsity);
-  nassertv(_num_entries == 0);
 
-  // Now copy the entries from the old table into the new table.
+  // Now copy the entries from the old table into the new table.  We don't
-  for (size_t i = 0; i < old_table_size; ++i) {
+  // have to reorder these, fortunately.  Hopefully, a smart compiler will
-    if (old_map.has_element(i)) {
+  // optimize this to a memcpy.
-      size_t new_index = get_hash(old_map._table[i]._key);
+  for (size_t i = 0; i < _num_entries; ++i) {
+    new(&_table[i]) TableEntry(move(old_table[i]));
+    old_table[i].~TableEntry();
+  }
 
-      while (exists_array[new_index] != 0) {
+  // We don't need this old thing anymore.
+  old_chain->deallocate(old_table, TypeHandle::none());
+
+  // Reindex the table.
+  for (size_t i = 0; i < _num_entries; ++i) {
+    size_t slot = get_hash(_table[i]._key);
+
+    while (has_slot(slot)) {
       // Hash conflict;  look for a better spot.  This has to succeed.
-        new_index = (new_index + 1) & (_table_size - 1);
+      slot = next_hash(slot);
-      }
-
-#ifdef USE_MOVE_SEMANTICS
-      // Use C++11 rvalue references to invoke the move constructor, which may
-      // be more efficient.
-      new(&_table[new_index]) TableEntry(move(old_map._table[i]));
-#else
-      new(&_table[new_index]) TableEntry(old_map._table[i]);
-#endif
-      exists_array[new_index] = true;
-      ++_num_entries;
     }
+    index_array[slot] = (int)i;
   }
 
   nassertv(validate());
-  nassertv(old_map.validate());
-
-  nassertv(_num_entries == old_map._num_entries);
 }

panda/src/putil/simpleHashMap.h

@@ -20,16 +20,25 @@
 
 /**
  * This template class implements an unordered map of keys to data,
- * implemented as a hashtable.  It is similar to STL's hash_map, but (a) it
+ * implemented as a hashtable.  It is similar to STL's hash_map, but
- * has a simpler interface (we don't mess around with iterators), (b) it wants
+ * (a) it has a simpler interface (we don't mess around with iterators),
- * an additional method on the Compare object, Compare::is_equal(a, b), and
+ * (b) it wants an additional method on the Compare object,
- * (c) it doesn't depend on the system STL providing hash_map.
+       Compare::is_equal(a, b),
+ * (c) it doesn't depend on the system STL providing hash_map,
+ * (d) it allows for efficient iteration over the entries,
+ * (e) permits removal and resizing during forward iteration, and
+ * (f) it has a constexpr constructor.
  */
 template<class Key, class Value, class Compare = method_hash<Key, less<Key> > >
 class SimpleHashMap {
+  // Per-entry overhead is determined by sizeof(int) * sparsity.  Should be a
+  // power of two.
+  static const unsigned int sparsity = 2u;
+
 public:
 #ifndef CPPPARSER
-  INLINE SimpleHashMap(const Compare &comp = Compare());
+  CONSTEXPR SimpleHashMap(const Compare &comp = Compare());
+  INLINE SimpleHashMap(SimpleHashMap &&from) NOEXCEPT;
   INLINE ~SimpleHashMap();
 
   INLINE void swap(SimpleHashMap &other);
@@ -40,9 +49,8 @@ public:
   void clear();
 
   INLINE Value &operator [] (const Key &key);
+  CONSTEXPR size_t size() const;
 
-  INLINE size_t get_size() const;
-  INLINE bool has_element(size_t n) const;
   INLINE const Key &get_key(size_t n) const;
   INLINE const Value &get_data(size_t n) const;
   INLINE Value &modify_data(size_t n);
@@ -62,11 +70,13 @@ private:
   class TableEntry;
 
   INLINE size_t get_hash(const Key &key) const;
+  INLINE size_t next_hash(size_t hash) const;
 
+  INLINE int find_slot(const Key &key) const;
+  INLINE bool has_slot(size_t slot) const;
   INLINE bool is_element(size_t n, const Key &key) const;
-  INLINE void store_new_element(size_t n, const Key &key, const Value &data);
+  INLINE size_t store_new_element(size_t n, const Key &key, const Value &data);
-  INLINE void clear_element(size_t n);
+  INLINE int *get_index_array() const;
-  INLINE unsigned char *get_exists_array() const;
 
   void new_table();
   INLINE bool consider_expand_table();