From ee2530ea5ff09dbd0d44c82cc075e02344cfa23b Mon Sep 17 00:00:00 2001 From: rdb Date: Mon, 29 May 2017 19:07:49 +0200 Subject: [PATCH 1/8] makepanda: changes for FreeBSD. Add instructions, and: - change package name from Panda3D to panda3d - change default compiler to clang on FreeBSD - fix ldconfig to point to panda3d lib dir [skip ci] --- README.md | 22 ++++++++++++++++++++++ makepanda/makepanda.py | 12 ++++++------ makepanda/makepandacore.py | 4 ++-- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index b87fd4ca10..0323d41b02 100644 --- a/README.md +++ b/README.md @@ -134,6 +134,28 @@ If the build was successful, makepanda will have generated a .dmg file in the source directory containing the installer. Simply open it and run the package file in order to install the SDK onto your system. +FreeBSD +------- + +Building on FreeBSD is very similar to building on Linux. You will need to +install the requisite packages using the system package manager. To install +the recommended set of dependencies, you can use this command: + +```bash +pkg install pkgconf png jpeg-turbo tiff freetype2 eigen squish openal opusfile libvorbis libX11 libGL ode bullet assimp openexr +``` + +You will also need to choose which version of Python you want to use. +Install the appropriate package for it (such as `python2` or `python36`) and +run the makepanda script with your chosen Python version: + +```bash +python3.6 makepanda/makepanda.py --everything --installer --no-egl --no-gles --no-gles2 +``` + +If successful, this will produce a .pkg file in the root of the source +directory which you can install using `pkg install`. + Reporting Issues ================ diff --git a/makepanda/makepanda.py b/makepanda/makepanda.py index b5044b1761..78779ca43b 100755 --- a/makepanda/makepanda.py +++ b/makepanda/makepanda.py @@ -6879,7 +6879,7 @@ Panda3D's intended game-development language is Python. The engine itself is wri This package contains the SDK for development with Panda3D, install panda3d-runtime for the runtime files. -WWW: http://www.panda3d.org/ +WWW: https://www.panda3d.org/ """ # FreeBSD pkg-descr @@ -6888,7 +6888,7 @@ Runtime binary and browser plugin for the Panda3D Game Engine This package contains the runtime distribution and browser plugin of the Panda3D engine. It allows you view webpages that contain Panda3D content and to run games created with Panda3D that are packaged as .p3d file. -WWW: http://www.panda3d.org/ +WWW: https://www.panda3d.org/ """ # FreeBSD PKG Manifest template file @@ -6897,8 +6897,8 @@ name: NAME version: VERSION arch: ARCH origin: ORIGIN -comment: "Panda 3D Engine" -www: http://www.panda3d.org +comment: "Panda3D free 3D engine SDK" +www: https://www.panda3d.org maintainer: rdb prefix: /usr/local flatsize: INSTSIZEMB @@ -7290,7 +7290,7 @@ def MakeInstallerFreeBSD(): plist_txt += os.path.join(root, f)[21:] + "\n" if not RUNTIME: - plist_txt += "@postexec /sbin/ldconfig -m /usr/local/lib\n" + plist_txt += "@postexec /sbin/ldconfig -m /usr/local/lib/panda3d\n" plist_txt += "@postunexec /sbin/ldconfig -R\n" for remdir in ("lib/panda3d", "share/panda3d", "include/panda3d"): @@ -7312,7 +7312,7 @@ def MakeInstallerFreeBSD(): if python_pkg: dependencies += python_pkg - manifest_txt = INSTALLER_PKG_MANIFEST_FILE[1:].replace("NAME", 'Panda3D' if not RUNTIME else 'Panda3D-Runtime') + manifest_txt = INSTALLER_PKG_MANIFEST_FILE[1:].replace("NAME", 'panda3d' if not RUNTIME else 'panda3d-runtime') manifest_txt = manifest_txt.replace("VERSION", VERSION) manifest_txt = manifest_txt.replace("ARCH", pkg_arch) manifest_txt = manifest_txt.replace("ORIGIN", 'devel/panda3d' if not RUNTIME else 'graphics/panda3d-runtime') diff --git a/makepanda/makepandacore.py b/makepanda/makepandacore.py index 7d86accc07..0999b5a42f 100644 --- a/makepanda/makepandacore.py +++ b/makepanda/makepandacore.py @@ -397,13 +397,13 @@ def CrossCompiling(): return GetTarget() != GetHost() def GetCC(): - if TARGET == 'darwin': + if TARGET == 'darwin' or TARGET == 'freebsd': return os.environ.get('CC', TOOLCHAIN_PREFIX + 'clang') else: return os.environ.get('CC', TOOLCHAIN_PREFIX + 'gcc') def GetCXX(): - if TARGET == 'darwin': + if TARGET == 'darwin' or TARGET == 'freebsd': return os.environ.get('CXX', TOOLCHAIN_PREFIX + 'clang++') else: return os.environ.get('CXX', TOOLCHAIN_PREFIX + 'g++') From 2e8d09a99c29e6a5d1c3ecd36519538aca114f33 Mon Sep 17 00:00:00 2001 From: rdb Date: Fri, 9 Jun 2017 19:48:04 +0200 Subject: [PATCH 2/8] Workaround for compiler bug when producing debug build (with thanks to consultit) --- panda/src/express/pointerTo.h | 4 ++-- panda/src/express/pointerToBase.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/panda/src/express/pointerTo.h b/panda/src/express/pointerTo.h index 1c4db74bea..15dbcdefb0 100644 --- a/panda/src/express/pointerTo.h +++ b/panda/src/express/pointerTo.h @@ -70,7 +70,7 @@ class PointerTo : public PointerToBase { public: typedef TYPENAME PointerToBase::To To; PUBLISHED: - CONSTEXPR PointerTo() NOEXCEPT DEFAULT_CTOR; + ALWAYS_INLINE CONSTEXPR PointerTo() NOEXCEPT DEFAULT_CTOR; ALWAYS_INLINE PointerTo(To *ptr) NOEXCEPT; INLINE PointerTo(const PointerTo ©); @@ -133,7 +133,7 @@ class ConstPointerTo : public PointerToBase { public: typedef TYPENAME PointerToBase::To To; PUBLISHED: - CONSTEXPR ConstPointerTo() NOEXCEPT DEFAULT_CTOR; + ALWAYS_INLINE CONSTEXPR ConstPointerTo() NOEXCEPT DEFAULT_CTOR; ALWAYS_INLINE ConstPointerTo(const To *ptr) NOEXCEPT; INLINE ConstPointerTo(const PointerTo ©); INLINE ConstPointerTo(const ConstPointerTo ©); diff --git a/panda/src/express/pointerToBase.h b/panda/src/express/pointerToBase.h index d19edd6b5c..4263312daa 100644 --- a/panda/src/express/pointerToBase.h +++ b/panda/src/express/pointerToBase.h @@ -31,7 +31,7 @@ public: typedef T To; protected: - CONSTEXPR PointerToBase() NOEXCEPT DEFAULT_CTOR; + ALWAYS_INLINE CONSTEXPR PointerToBase() NOEXCEPT DEFAULT_CTOR; INLINE PointerToBase(To *ptr); INLINE PointerToBase(const PointerToBase ©); INLINE ~PointerToBase(); From 6b882ac767704d4843db72988ef8229cb935fd33 Mon Sep 17 00:00:00 2001 From: rdb Date: Fri, 9 Jun 2017 20:12:49 +0200 Subject: [PATCH 3/8] Fix slow garbage collect after using a lot of states --- panda/src/pgraph/renderAttrib.cxx | 4 ++++ panda/src/pgraph/renderState.cxx | 4 ++++ panda/src/pgraph/transformState.cxx | 4 ++++ panda/src/putil/simpleHashMap.I | 35 +++++++++++++++++++++++------ panda/src/putil/simpleHashMap.h | 4 +++- 5 files changed, 43 insertions(+), 8 deletions(-) diff --git a/panda/src/pgraph/renderAttrib.cxx b/panda/src/pgraph/renderAttrib.cxx index 3a7b511381..f2425a32ff 100644 --- a/panda/src/pgraph/renderAttrib.cxx +++ b/panda/src/pgraph/renderAttrib.cxx @@ -255,6 +255,10 @@ garbage_collect() { nassertr(_attribs->validate(), 0); #endif + // If we just cleaned up a lot of attribs, see if we can reduce the table in + // size. This will help reduce iteration overhead in the future. + _attribs->consider_shrink_table(); + size_t new_size = _attribs->get_num_entries(); return orig_size - new_size; } diff --git a/panda/src/pgraph/renderState.cxx b/panda/src/pgraph/renderState.cxx index d8cd5a6dd3..99e02792b2 100644 --- a/panda/src/pgraph/renderState.cxx +++ b/panda/src/pgraph/renderState.cxx @@ -1001,6 +1001,10 @@ garbage_collect() { nassertr(_states->validate(), 0); #endif + // If we just cleaned up a lot of states, see if we can reduce the table in + // size. This will help reduce iteration overhead in the future. + _states->consider_shrink_table(); + int new_size = _states->get_num_entries(); return orig_size - new_size + num_attribs; } diff --git a/panda/src/pgraph/transformState.cxx b/panda/src/pgraph/transformState.cxx index be1ed4a15a..765dafb96a 100644 --- a/panda/src/pgraph/transformState.cxx +++ b/panda/src/pgraph/transformState.cxx @@ -1253,6 +1253,10 @@ garbage_collect() { nassertr(_states->validate(), 0); #endif + // If we just cleaned up a lot of states, see if we can reduce the table in + // size. This will help reduce iteration overhead in the future. + _states->consider_shrink_table(); + int new_size = _states->get_num_entries(); return orig_size - new_size; } diff --git a/panda/src/putil/simpleHashMap.I b/panda/src/putil/simpleHashMap.I index 74245fd606..4b21b02d1e 100644 --- a/panda/src/putil/simpleHashMap.I +++ b/panda/src/putil/simpleHashMap.I @@ -522,27 +522,48 @@ new_table() { template INLINE bool SimpleHashMap:: consider_expand_table() { - if (_num_entries >= (_table_size >> 1)) { - expand_table(); + if (_num_entries < (_table_size >> 1)) { + return false; + } else { + resize_table(_table_size << 1); return true; } - return false; } /** - * Doubles the size of the existing table. + * Shrinks the table if the allocated storage is significantly larger than the + * number of elements in it. Returns true if shrunk, false otherwise. + */ +template +INLINE bool SimpleHashMap:: +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. + if (_table_size <= 8 || (_table_size >> 3) < _num_entries) { + return false; + } else { + size_t new_size = _table_size; + do { + new_size >>= 1; + } while ((new_size >> 3) >= _num_entries); + resize_table(new_size); + return true; + } +} + +/** + * Resizes the existing table. */ template void SimpleHashMap:: -expand_table() { +resize_table(size_t new_size) { nassertv(_table_size != 0); SimpleHashMap old_map(_comp); swap(old_map); - // Double the table size. size_t old_table_size = old_map._table_size; - _table_size = (old_table_size << 1); + _table_size = new_size; nassertv(_table == NULL); // We allocate enough bytes for _table_size elements of TableEntry, plus diff --git a/panda/src/putil/simpleHashMap.h b/panda/src/putil/simpleHashMap.h index c1a25fc7f9..928b704e05 100644 --- a/panda/src/putil/simpleHashMap.h +++ b/panda/src/putil/simpleHashMap.h @@ -56,6 +56,8 @@ public: void write(ostream &out) const; bool validate() const; + INLINE bool consider_shrink_table(); + private: class TableEntry; @@ -68,7 +70,7 @@ private: void new_table(); INLINE bool consider_expand_table(); - void expand_table(); + void resize_table(size_t new_size); class TableEntry { public: From 59c435c487716e769858ac3d068b39d9aa66361d Mon Sep 17 00:00:00 2001 From: rdb Date: Fri, 9 Jun 2017 20:17:30 +0200 Subject: [PATCH 4/8] Add FreeBSD package-related files to .gitignore --- .gitignore | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.gitignore b/.gitignore index 44373e05fe..599bb0fd9f 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,7 @@ core core.* vgcore.* +*.core # Editor files/directories *.save @@ -22,6 +23,9 @@ vgcore.* *.o *.gch *.pch +/+DESC +/+MANIFEST +/pkg-plist # Produced installer/executables /*.exe @@ -31,6 +35,7 @@ vgcore.* /*.pkg /*.dmg /*.whl +/*.txz # CMake /build/ From 88e963b936b96f9e262d4642377f26fd6fabc9ac Mon Sep 17 00:00:00 2001 From: rdb Date: Fri, 9 Jun 2017 20:36:45 +0200 Subject: [PATCH 5/8] Fix compilation errors in GCC 4.6 and 4.8 --- dtool/src/dtoolbase/dtoolbase_cc.h | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/dtool/src/dtoolbase/dtoolbase_cc.h b/dtool/src/dtoolbase/dtoolbase_cc.h index 88d4244af6..cd6e76b4fe 100644 --- a/dtool/src/dtoolbase/dtoolbase_cc.h +++ b/dtool/src/dtoolbase/dtoolbase_cc.h @@ -178,12 +178,6 @@ template typename remove_reference::type &&move(T &&t) { # endif #elif defined(__GNUC__) && (__cplusplus >= 201103L) // GCC -// GCC defines several macros which we can query. List of all supported -// builtin macros: https://gcc.gnu.org/projects/cxx-status.html -# if __cpp_constexpr >= 200704 -# define CONSTEXPR constexpr -# endif - // Starting at GCC 4.4 # if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 4) # define DEFAULT_CTOR = default @@ -194,6 +188,7 @@ template typename remove_reference::type &&move(T &&t) { // Starting at GCC 4.6 # if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6) +# define CONSTEXPR constexpr # define NOEXCEPT noexcept # define USE_MOVE_SEMANTICS # define FINAL final @@ -205,6 +200,12 @@ template typename remove_reference::type &&move(T &&t) { # define OVERRIDE override # endif +// GCC defines several macros which we can query. List of all supported +// builtin macros: https://gcc.gnu.org/projects/cxx-status.html +# if !defined(CONSTEXPR) && __cpp_constexpr >= 200704 +# define CONSTEXPR constexpr +# endif + #elif defined(_MSC_VER) && _MSC_VER >= 1900 // Visual Studio 2015 # define CONSTEXPR constexpr # define NOEXCEPT noexcept From ca2e96d296eea31670819b1eb11914299a771c72 Mon Sep 17 00:00:00 2001 From: rdb Date: Mon, 12 Jun 2017 19:46:59 +0200 Subject: [PATCH 6/8] Fix for certain libjpeg versions that override INLINE macro --- panda/src/pnmimagetypes/pnmFileTypeJPG.h | 9 +++++++++ panda/src/vision/webcamVideoCursorV4L.h | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/panda/src/pnmimagetypes/pnmFileTypeJPG.h b/panda/src/pnmimagetypes/pnmFileTypeJPG.h index 2dcd506322..b82534189a 100644 --- a/panda/src/pnmimagetypes/pnmFileTypeJPG.h +++ b/panda/src/pnmimagetypes/pnmFileTypeJPG.h @@ -36,6 +36,10 @@ #include #endif +// jconfig.h overrides our INLINE definition. +#ifdef __GNUC__ +#pragma push_macro("INLINE") +#endif extern "C" { #include // jpeglib requires this to be included first. @@ -43,6 +47,11 @@ extern "C" { #include } +// Restore our own INLINE definition. +#ifdef __GNUC__ +#pragma pop_macro("INLINE") +#endif + /** * For reading and writing Jpeg files. */ diff --git a/panda/src/vision/webcamVideoCursorV4L.h b/panda/src/vision/webcamVideoCursorV4L.h index 7eb7b02763..9555096eea 100644 --- a/panda/src/vision/webcamVideoCursorV4L.h +++ b/panda/src/vision/webcamVideoCursorV4L.h @@ -24,9 +24,18 @@ #include #ifdef HAVE_JPEG +// jconfig.h overrides our INLINE definition. +#ifdef __GNUC__ +#pragma push_macro("INLINE") +#endif + extern "C" { #include } + +#ifdef __GNUC__ +#pragma pop_macro("INLINE") +#endif #endif class WebcamVideoV4L; From a66b497f2b6d5e06012d9aee7a2b9286c8d40681 Mon Sep 17 00:00:00 2001 From: rdb Date: Mon, 12 Jun 2017 19:47:54 +0200 Subject: [PATCH 7/8] We no longer support GCC 4.6. Version 4.8+ is recommended. Ubuntu Precise users can install GCC 4.8 from the ppa:ubuntu-toolchain-r/test repository. CentOS 5/6 users can install devtoolset-2 or later. --- .travis.yml | 11 +++++++++-- dtool/src/dtoolbase/dtoolbase_cc.h | 4 ++-- makepanda/makepanda.py | 2 +- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index c8176d293b..7d8a3003b6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,15 +2,22 @@ language: cpp sudo: false matrix: include: - - compiler: gcc - env: PYTHONV=python2.7 FLAGS=--optimize=4 - compiler: clang env: PYTHONV=python3 FLAGS=--installer - compiler: clang env: PYTHONV=python2.7 FLAGS=--override=STDFLOAT_DOUBLE=1 + - compiler: gcc + env: PYTHONV=python2.7 FLAGS=--optimize=4 + before_install: + - export CC=gcc-4.7 + - export CXX=g++-4.7 addons: apt: + sources: + - ubuntu-toolchain-r-test packages: + - gcc-4.7 + - g++-4.7 - bison - flex - libfreetype6-dev diff --git a/dtool/src/dtoolbase/dtoolbase_cc.h b/dtool/src/dtoolbase/dtoolbase_cc.h index cd6e76b4fe..f4fb89015c 100644 --- a/dtool/src/dtoolbase/dtoolbase_cc.h +++ b/dtool/src/dtoolbase/dtoolbase_cc.h @@ -176,7 +176,7 @@ template typename remove_reference::type &&move(T &&t) { # if __has_extension(cxx_deleted_functions) # define DELETED = delete # endif -#elif defined(__GNUC__) && (__cplusplus >= 201103L) // GCC +#elif defined(__GNUC__) // GCC // Starting at GCC 4.4 # if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 4) @@ -191,12 +191,12 @@ template typename remove_reference::type &&move(T &&t) { # define CONSTEXPR constexpr # define NOEXCEPT noexcept # define USE_MOVE_SEMANTICS -# define FINAL final # define MOVE(x) move(x) # endif // Starting at GCC 4.7 # if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 7) +# define FINAL final # define OVERRIDE override # endif diff --git a/makepanda/makepanda.py b/makepanda/makepanda.py index 78779ca43b..a2df279c0d 100755 --- a/makepanda/makepanda.py +++ b/makepanda/makepanda.py @@ -1214,7 +1214,7 @@ def CompileCxx(obj,src,opts): if (COMPILER=="GCC"): if (src.endswith(".c")): cmd = GetCC() +' -fPIC -c -o ' + obj - else: cmd = GetCXX()+' -std=gnu++0x -ftemplate-depth-70 -fPIC -c -o ' + obj + else: cmd = GetCXX()+' -std=gnu++11 -ftemplate-depth-70 -fPIC -c -o ' + obj for (opt, dir) in INCDIRECTORIES: if (opt=="ALWAYS") or (opt in opts): cmd += ' -I' + BracketNameWithQuotes(dir) for (opt, dir) in FRAMEWORKDIRECTORIES: From 90f96fc1a9f6f49c8a15aa935d7faef401d1d895 Mon Sep 17 00:00:00 2001 From: rdb Date: Mon, 12 Jun 2017 19:55:35 +0200 Subject: [PATCH 8/8] 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 | 75 ++-- panda/src/pgraph/renderState.I | 16 +- panda/src/pgraph/renderState.cxx | 314 +++++++-------- panda/src/pgraph/renderState_ext.cxx | 73 ++-- panda/src/pgraph/transformState.I | 16 +- panda/src/pgraph/transformState.cxx | 321 +++++++--------- panda/src/pgraph/transformState_ext.cxx | 30 +- panda/src/putil/simpleHashMap.I | 487 +++++++++++++----------- panda/src/putil/simpleHashMap.h | 30 +- 9 files changed, 641 insertions(+), 721 deletions(-) diff --git a/panda/src/pgraph/renderAttrib.cxx b/panda/src/pgraph/renderAttrib.cxx index f2425a32ff..3b8c68ad0c 100644 --- a/panda/src/pgraph/renderAttrib.cxx +++ b/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_size(); + size_t size = _attribs->get_num_entries(); + 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,40 +214,51 @@ 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(); - int num_this_pass = int(size * garbage_collect_states_rate); + size_t size = orig_size; + 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 - // we stored it in the cache). Now it's time to delete it. This is - // safe, because we're holding the _attribs_lock, so it's not possible - // for some other thread to find the attrib in the cache and ref it - // while we're doing this. - attrib->release_new(); - unref_delete(attrib); - } + 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 + // we stored it in the cache). Now it's time to delete it. This is + // safe, because we're holding the _attribs_lock, so it's not possible + // for some other thread to find the attrib in the cache and ref it + // 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 orig_size - new_size; + return (int)orig_size - (int)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)); } } diff --git a/panda/src/pgraph/renderState.I b/panda/src/pgraph/renderState.I index 75b6f1070a..185b18b24d 100644 --- a/panda/src/pgraph/renderState.I +++ b/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; } diff --git a/panda/src/pgraph/renderState.cxx b/panda/src/pgraph/renderState.cxx index 99e02792b2..8faef8676c 100644 --- a/panda/src/pgraph/renderState.cxx +++ b/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,40 +782,33 @@ get_num_unused_states() { typedef pmap 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; - int cache_size = state->_composition_cache.get_size(); + size_t i; + 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. - pair ir = - state_count.insert(StateCount::value_type(result, 1)); - if (!ir.second) { - // If the above insert operation fails, then it's already in the - // cache; increment its value. - (*(ir.first)).second++; - } + 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. + pair ir = + state_count.insert(StateCount::value_type(result, 1)); + if (!ir.second) { + // If the above insert operation fails, then it's already in the + // cache; increment its value. + (*(ir.first)).second++; } } } - 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 RenderState *result = state->_invert_composition_cache.get_data(i)._result; - if (result != (const RenderState *)NULL && result != state) { - pair ir = - state_count.insert(StateCount::value_type(result, 1)); - if (!ir.second) { - (*(ir.first)).second++; - } + const RenderState *result = state->_invert_composition_cache.get_data(i)._result; + if (result != (const RenderState *)NULL && result != state) { + pair ir = + state_count.insert(StateCount::value_type(result, 1)); + if (!ir.second) { + (*(ir.first)).second++; } } } @@ -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,28 +884,24 @@ clear_cache() { for (ti = temp_states.begin(); ti != temp_states.end(); ++ti) { RenderState *state = (RenderState *)(*ti).p(); - int i; - int cache_size = (int)state->_composition_cache.get_size(); + size_t i; + 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); - } + 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); - } + 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()); @@ -950,53 +936,62 @@ 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 num_this_pass = int((int)size * garbage_collect_states_rate); - if (size <= 0 || num_this_pass <= 0) { + size_t size = orig_size; + size_t num_this_pass = max(0, int(size * garbage_collect_states_rate)); + 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 && - 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 RenderState and break it if it - // exists. - state->detect_and_break_cycles(); - } - } - - if (state->get_ref_count() == 1) { - // This state has recently been unreffed to 1 (the one we added when - // we stored it in the cache). 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(); - state->cache_unref(); - delete state; + RenderState *state = (RenderState *)_states->get_key(si); + if (break_and_uniquify) { + 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 RenderState and break it if it + // exists. + state->detect_and_break_cycles(); } } - si = (si + 1) & (size - 1); + if (state->get_ref_count() == 1) { + // This state has recently been unreffed to 1 (the one we added when + // we stored it in the cache). 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(); + 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; } 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 orig_size - new_size + num_attribs; + return (int)orig_size - (int)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_size(); + size_t size = _states->get_num_entries(); + out << size << " states:\n"; 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,41 +1568,37 @@ r_detect_cycles(const RenderState *start_state, } ((RenderState *)current_state)->_cycle_detect = this_seq; - int i; - int cache_size = current_state->_composition_cache.get_size(); + size_t i; + 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, - this_seq, cycle_desc)) { - // Cycle detected. - if (cycle_desc != (CompositionCycleDesc *)NULL) { - const RenderState *other = current_state->_composition_cache.get_key(i); - CompositionCycleDescEntry entry(other, result, false); - cycle_desc->push_back(entry); - } - return true; + 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, + this_seq, cycle_desc)) { + // Cycle detected. + if (cycle_desc != (CompositionCycleDesc *)NULL) { + const RenderState *other = current_state->_composition_cache.get_key(i); + CompositionCycleDescEntry entry(other, result, false); + cycle_desc->push_back(entry); } + return true; } } } - 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, - this_seq, cycle_desc)) { - // Cycle detected. - if (cycle_desc != (CompositionCycleDesc *)NULL) { - const RenderState *other = current_state->_invert_composition_cache.get_key(i); - CompositionCycleDescEntry entry(other, result, true); - cycle_desc->push_back(entry); - } - return true; + 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, + this_seq, cycle_desc)) { + // Cycle detected. + if (cycle_desc != (CompositionCycleDesc *)NULL) { + const RenderState *other = current_state->_invert_composition_cache.get_key(i); + CompositionCycleDescEntry entry(other, result, true); + cycle_desc->push_back(entry); } + return true; } } } @@ -1656,52 +1627,48 @@ r_detect_reverse_cycles(const RenderState *start_state, } ((RenderState *)current_state)->_cycle_detect = this_seq; - int i; - int cache_size = current_state->_composition_cache.get_size(); + size_t i; + 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); - nassertr(oi != -1, false); + const RenderState *other = current_state->_composition_cache.get_key(i); + if (other != current_state) { + int oi = other->_composition_cache.find(current_state); + nassertr(oi != -1, false); - const RenderState *result = other->_composition_cache.get_data(oi)._result; - if (result != (const RenderState *)NULL) { - if (r_detect_reverse_cycles(start_state, result, length + 1, - this_seq, cycle_desc)) { - // Cycle detected. - if (cycle_desc != (CompositionCycleDesc *)NULL) { - const RenderState *other = current_state->_composition_cache.get_key(i); - CompositionCycleDescEntry entry(other, result, false); - cycle_desc->push_back(entry); - } - return true; + const RenderState *result = other->_composition_cache.get_data(oi)._result; + if (result != (const RenderState *)NULL) { + if (r_detect_reverse_cycles(start_state, result, length + 1, + this_seq, cycle_desc)) { + // Cycle detected. + if (cycle_desc != (CompositionCycleDesc *)NULL) { + const RenderState *other = current_state->_composition_cache.get_key(i); + CompositionCycleDescEntry entry(other, result, false); + cycle_desc->push_back(entry); } + return true; } } } } - 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); - nassertr(oi != -1, false); + const RenderState *other = current_state->_invert_composition_cache.get_key(i); + if (other != current_state) { + int oi = other->_invert_composition_cache.find(current_state); + nassertr(oi != -1, false); - const RenderState *result = other->_invert_composition_cache.get_data(oi)._result; - if (result != (const RenderState *)NULL) { - if (r_detect_reverse_cycles(start_state, result, length + 1, - this_seq, cycle_desc)) { - // Cycle detected. - if (cycle_desc != (CompositionCycleDesc *)NULL) { - const RenderState *other = current_state->_invert_composition_cache.get_key(i); - CompositionCycleDescEntry entry(other, result, false); - cycle_desc->push_back(entry); - } - return true; + const RenderState *result = other->_invert_composition_cache.get_data(oi)._result; + if (result != (const RenderState *)NULL) { + if (r_detect_reverse_cycles(start_state, result, length + 1, + this_seq, cycle_desc)) { + // Cycle detected. + if (cycle_desc != (CompositionCycleDesc *)NULL) { + const RenderState *other = current_state->_invert_composition_cache.get_key(i); + CompositionCycleDescEntry entry(other, result, false); + cycle_desc->push_back(entry); } + return true; } } } @@ -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); diff --git a/panda/src/pgraph/renderState_ext.cxx b/panda/src/pgraph/renderState_ext.cxx index 5a7288f518..0dc0d8acc5 100644 --- a/panda/src/pgraph/renderState_ext.cxx +++ b/panda/src/pgraph/renderState_ext.cxx @@ -30,36 +30,29 @@ PyObject *Extension:: 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)) { + const RenderState *source = _this->_composition_cache.get_key(i); + if (source == (RenderState *)NULL) { a = Py_None; Py_INCREF(a); + } else { + source->ref(); + a = DTool_CreatePyInstanceTyped((void *)source, Dtool_RenderState, + true, true, source->get_type_index()); + } + const RenderState *result = _this->_composition_cache.get_data(i)._result; + if (result == (RenderState *)NULL) { b = Py_None; Py_INCREF(b); } else { - const RenderState *source = _this->_composition_cache.get_key(i); - if (source == (RenderState *)NULL) { - a = Py_None; - Py_INCREF(a); - } else { - source->ref(); - a = DTool_CreatePyInstanceTyped((void *)source, Dtool_RenderState, - true, true, source->get_type_index()); - } - const RenderState *result = _this->_composition_cache.get_data(i)._result; - if (result == (RenderState *)NULL) { - b = Py_None; - Py_INCREF(b); - } else { - result->ref(); - b = DTool_CreatePyInstanceTyped((void *)result, Dtool_RenderState, - true, true, result->get_type_index()); - } + result->ref(); + 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,36 +78,29 @@ PyObject *Extension:: 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)) { + const RenderState *source = _this->_invert_composition_cache.get_key(i); + if (source == (RenderState *)NULL) { a = Py_None; Py_INCREF(a); + } else { + source->ref(); + a = DTool_CreatePyInstanceTyped((void *)source, Dtool_RenderState, + true, true, source->get_type_index()); + } + const RenderState *result = _this->_invert_composition_cache.get_data(i)._result; + if (result == (RenderState *)NULL) { b = Py_None; Py_INCREF(b); } else { - const RenderState *source = _this->_invert_composition_cache.get_key(i); - if (source == (RenderState *)NULL) { - a = Py_None; - Py_INCREF(a); - } else { - source->ref(); - a = DTool_CreatePyInstanceTyped((void *)source, Dtool_RenderState, - true, true, source->get_type_index()); - } - const RenderState *result = _this->_invert_composition_cache.get_data(i)._result; - if (result == (RenderState *)NULL) { - b = Py_None; - Py_INCREF(b); - } else { - result->ref(); - b = DTool_CreatePyInstanceTyped((void *)result, Dtool_RenderState, - true, true, result->get_type_index()); - } + result->ref(); + 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(); - for (int si = 0; si < size; ++si) { - if (!RenderState::_states->has_element(si)) { - continue; - } + size_t size = RenderState::_states->get_num_entries(); + for (size_t si = 0; si < size; ++si) { const RenderState *state = RenderState::_states->get_key(si); state->ref(); PyObject *a = diff --git a/panda/src/pgraph/transformState.I b/panda/src/pgraph/transformState.I index 3decc64a51..474b78891f 100644 --- a/panda/src/pgraph/transformState.I +++ b/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; } diff --git a/panda/src/pgraph/transformState.cxx b/panda/src/pgraph/transformState.cxx index 765dafb96a..ae0d99b794 100644 --- a/panda/src/pgraph/transformState.cxx +++ b/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(); - for (int i = 0; i < size; ++i) { - if (!_invert_composition_cache.has_element(i)) { - continue; - } + size = _invert_composition_cache.get_num_entries(); + for (size_t i = 0; i < size; ++i) { 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,40 +1031,33 @@ get_num_unused_states() { typedef pmap 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. - pair ir = - state_count.insert(StateCount::value_type(result, 1)); - if (!ir.second) { - // If the above insert operation fails, then it's already in the - // cache; increment its value. - (*(ir.first)).second++; - } + 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. + pair ir = + state_count.insert(StateCount::value_type(result, 1)); + if (!ir.second) { + // If the above insert operation fails, then it's already in the + // cache; increment its value. + (*(ir.first)).second++; } } } - 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) { - pair ir = - state_count.insert(StateCount::value_type(result, 1)); - if (!ir.second) { - (*(ir.first)).second++; - } + const TransformState *result = state->_invert_composition_cache.get_data(i)._result; + if (result != (const TransformState *)NULL && result != state) { + pair ir = + state_count.insert(StateCount::value_type(result, 1)); + if (!ir.second) { + (*(ir.first)).second++; } } } @@ -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,28 +1134,24 @@ clear_cache() { for (ti = temp_states.begin(); ti != temp_states.end(); ++ti) { TransformState *state = (TransformState *)(*ti).p(); - int i; - int cache_size = (int)state->_composition_cache.get_size(); + size_t i; + 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); - } + 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); - } + 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()); @@ -1199,56 +1179,65 @@ 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 num_this_pass = int(size * garbage_collect_states_rate); - if (size <= 0 || num_this_pass <= 0) { + size_t size = orig_size; + size_t num_this_pass = max(0, int(size * garbage_collect_states_rate)); + 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 && - 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() == 1) { - // This state has recently been unreffed to 1 (the one we added when - // we stored it in the cache). 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(); - state->cache_unref(); - delete state; + TransformState *state = (TransformState *)_states->get_key(si); + if (break_and_uniquify) { + 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(); } } - si = (si + 1) & (size - 1); + if (state->get_ref_count() == 1) { + // This state has recently been unreffed to 1 (the one we added when + // we stored it in the cache). 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(); + 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; } 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 orig_size - new_size; + return (int)orig_size - (int)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_size(); + size_t size = _states->get_num_entries(); + out << size << " states:\n"; 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,41 +1777,37 @@ r_detect_cycles(const TransformState *start_state, } ((TransformState *)current_state)->_cycle_detect = this_seq; - int i; - int cache_size = current_state->_composition_cache.get_size(); + size_t i; + 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, - this_seq, cycle_desc)) { - // Cycle detected. - if (cycle_desc != (CompositionCycleDesc *)NULL) { - const TransformState *other = current_state->_composition_cache.get_key(i); - CompositionCycleDescEntry entry(other, result, false); - cycle_desc->push_back(entry); - } - return true; + 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, + this_seq, cycle_desc)) { + // Cycle detected. + if (cycle_desc != (CompositionCycleDesc *)NULL) { + const TransformState *other = current_state->_composition_cache.get_key(i); + CompositionCycleDescEntry entry(other, result, false); + cycle_desc->push_back(entry); } + return true; } } } - 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, - this_seq, cycle_desc)) { - // Cycle detected. - if (cycle_desc != (CompositionCycleDesc *)NULL) { - const TransformState *other = current_state->_invert_composition_cache.get_key(i); - CompositionCycleDescEntry entry(other, result, true); - cycle_desc->push_back(entry); - } - return true; + 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, + this_seq, cycle_desc)) { + // Cycle detected. + if (cycle_desc != (CompositionCycleDesc *)NULL) { + const TransformState *other = current_state->_invert_composition_cache.get_key(i); + CompositionCycleDescEntry entry(other, result, true); + cycle_desc->push_back(entry); } + return true; } } } @@ -1868,52 +1836,48 @@ r_detect_reverse_cycles(const TransformState *start_state, } ((TransformState *)current_state)->_cycle_detect = this_seq; - int i; - int cache_size = current_state->_composition_cache.get_size(); + size_t i; + 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); - nassertr(oi != -1, false); + const TransformState *other = current_state->_composition_cache.get_key(i); + if (other != current_state) { + int oi = other->_composition_cache.find(current_state); + nassertr(oi != -1, false); - const TransformState *result = other->_composition_cache.get_data(oi)._result; - if (result != (const TransformState *)NULL) { - if (r_detect_reverse_cycles(start_state, result, length + 1, - this_seq, cycle_desc)) { - // Cycle detected. - if (cycle_desc != (CompositionCycleDesc *)NULL) { - const TransformState *other = current_state->_composition_cache.get_key(i); - CompositionCycleDescEntry entry(other, result, false); - cycle_desc->push_back(entry); - } - return true; + const TransformState *result = other->_composition_cache.get_data(oi)._result; + if (result != (const TransformState *)NULL) { + if (r_detect_reverse_cycles(start_state, result, length + 1, + this_seq, cycle_desc)) { + // Cycle detected. + if (cycle_desc != (CompositionCycleDesc *)NULL) { + const TransformState *other = current_state->_composition_cache.get_key(i); + CompositionCycleDescEntry entry(other, result, false); + cycle_desc->push_back(entry); } + return true; } } } } - 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); - nassertr(oi != -1, false); + const TransformState *other = current_state->_invert_composition_cache.get_key(i); + if (other != current_state) { + int oi = other->_invert_composition_cache.find(current_state); + nassertr(oi != -1, false); - const TransformState *result = other->_invert_composition_cache.get_data(oi)._result; - if (result != (const TransformState *)NULL) { - if (r_detect_reverse_cycles(start_state, result, length + 1, - this_seq, cycle_desc)) { - // Cycle detected. - if (cycle_desc != (CompositionCycleDesc *)NULL) { - const TransformState *other = current_state->_invert_composition_cache.get_key(i); - CompositionCycleDescEntry entry(other, result, false); - cycle_desc->push_back(entry); - } - return true; + const TransformState *result = other->_invert_composition_cache.get_data(oi)._result; + if (result != (const TransformState *)NULL) { + if (r_detect_reverse_cycles(start_state, result, length + 1, + this_seq, cycle_desc)) { + // Cycle detected. + if (cycle_desc != (CompositionCycleDesc *)NULL) { + const TransformState *other = current_state->_invert_composition_cache.get_key(i); + CompositionCycleDescEntry entry(other, result, false); + cycle_desc->push_back(entry); } + return true; } } } @@ -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); diff --git a/panda/src/pgraph/transformState_ext.cxx b/panda/src/pgraph/transformState_ext.cxx index e037cd2a16..44ddfd71f8 100644 --- a/panda/src/pgraph/transformState_ext.cxx +++ b/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(); - for (int si = 0; si < size; ++si) { - if (!_this->_composition_cache.has_element(si)) { - continue; - } - + size_t size = _this->_composition_cache.get_num_entries(); + for (size_t si = 0; si < size; ++si) { 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(); - for (int si = 0; si < size; ++si) { - if (!_this->_invert_composition_cache.has_element(si)) { - continue; - } - + size_t size = _this->_invert_composition_cache.get_num_entries(); + for (size_t si = 0; si < size; ++si) { 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(); - for (int si = 0; si < size; ++si) { - if (!TransformState::_states->has_element(si)) { - continue; - } + size_t size = TransformState::_states->get_num_entries(); + for (size_t si = 0; si < size; ++si) { 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(); - for (int si = 0; si < size; ++si) { - if (!TransformState::_states->has_element(si)) { - continue; - } + size_t size = TransformState::_states->get_num_entries(); + for (size_t si = 0; si < size; ++si) { const TransformState *state = TransformState::_states->get_key(si); if (state->get_cache_ref_count() == state->get_ref_count()) { state->ref(); diff --git a/panda/src/putil/simpleHashMap.I b/panda/src/putil/simpleHashMap.I index 4b21b02d1e..5f797dfde1 100644 --- a/panda/src/putil/simpleHashMap.I +++ b/panda/src/putil/simpleHashMap.I @@ -15,16 +15,31 @@ * */ template -INLINE SimpleHashMap:: +CONSTEXPR SimpleHashMap:: SimpleHashMap(const Compare &comp) : - _table(NULL), - _deleted_chain(NULL), + _table(nullptr), + _deleted_chain(nullptr), _table_size(0), _num_entries(0), _comp(comp) { } +/** + * + */ +template +INLINE SimpleHashMap:: +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,29 +84,13 @@ find(const Key &key) const { return -1; } - size_t index = get_hash(key); - if (!has_element(index)) { + int slot = find_slot(key); + if (slot >= 0) { + return get_index_array()[slot]; + } else { + // The key is not in the table. return -1; } - 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; } /** @@ -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); - store_new_element(index, key, data); - ++_num_entries; + int pos = store_new_element(get_hash(key), key, data); #ifdef _DEBUG - nassertr(validate(), index); + nassertr(validate(), pos); #endif - return index; + return pos; } + consider_expand_table(); - size_t index = get_hash(key); - if (!has_element(index)) { + const int *index_array = get_index_array(); + 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); - ++_num_entries; + index = store_new_element(hash, key, data); #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; - i = (i + 1) & (_table_size - 1); - while (i != index) { - if (!has_element(i)) { + size_t slot = next_hash(hash); + while (slot != hash) { + index = index_array[slot]; + if (index < 0) { if (consider_expand_table()) { return store(key, data); } - store_new_element(i, key, data); - ++_num_entries; + index = store_new_element(slot, key, data); #ifdef _DEBUG - nassertr(validate(), i); + nassertr(validate(), index); #endif - return i; + return index; } - if (is_element(i, key)) { - _table[i]._data = data; + if (is_element(index, key)) { + _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 INLINE bool SimpleHashMap:: remove(const Key &key) { - int index = find(key); - if (index == -1) { + if (_num_entries == 0) { + // 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 void SimpleHashMap:: clear() { if (_table_size != 0) { - for (size_t i = 0; i < _table_size; ++i) { - if (has_element(i)) { - clear_element(i); - } + for (size_t i = 0; i < _num_entries; ++i) { + _table[i].~TableEntry(); } _deleted_chain->deallocate(_table, TypeHandle::none()); - _table = NULL; - _deleted_chain = NULL; + _table = nullptr; + _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 -INLINE size_t SimpleHashMap:: -get_size() const { - return _table_size; +CONSTEXPR size_t SimpleHashMap:: +size() const { + return _num_entries; } /** - * Returns true if there is an element stored in the nth slot, false - * otherwise. + * Returns the key in the nth entry of the table. * - * n should be in the range 0 <= n < get_size(). - */ -template -INLINE bool SimpleHashMap:: -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(). + * @param n should be in the range 0 <= n < size(). */ template INLINE const Key &SimpleHashMap:: 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 - * has_element() to check this first). n should be in the range 0 <= n < - * get_size(). + * @param n should be in the range 0 <= n < size(). */ template INLINE const Value &SimpleHashMap:: 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 - * has_element() to check this first). n should be in the range 0 <= n < - * get_size(). + * @param n should be in the range 0 <= n < size(). */ template INLINE Value &SimpleHashMap:: 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 - * has_element() to check this first). n should be in the range 0 <= n < - * get_size(). + * @param n should be in the range 0 <= n < size(). */ template INLINE void SimpleHashMap:: 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 - * has_element() to check this first). n should be in the range 0 <= n < - * get_size(). + * @param n should be in the range 0 <= n < size(). */ template void SimpleHashMap:: remove_element(size_t n) { - nassertv(has_element(n)); - - 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 + nassertv(n < _num_entries); + remove(_table[n]._key); } /** - * Returns the number of active entries in the table. This is not necessarily - * 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(). + * Returns the number of active entries in the table. Same as size(). */ template INLINE size_t SimpleHashMap:: @@ -352,7 +361,7 @@ get_num_entries() const { } /** - * Returns true if the table is empty; i.e. get_num_entries() == 0. + * Returns true if the table is empty; i.e. get_num_entries() == 0. */ template INLINE bool SimpleHashMap:: @@ -367,17 +376,20 @@ template void SimpleHashMap:: output(ostream &out) const { out << "SimpleHashMap (" << _num_entries << " entries): ["; - for (size_t i = 0; i < _table_size; ++i) { - if (!has_element(i)) { + const int *index_array = get_index_array(); + 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 = get_hash(_table[i]._key); - if (index != i) { + size_t index = (size_t)index_array[slot]; + out << " " << index; + 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:: 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:: validate() const { size_t count = 0; - const unsigned char *exists_array = get_exists_array(); - - for (size_t i = 0; i < _table_size; ++i) { - if (exists_array[i] != 0) { + const int *index_array = get_index_array(); + size_t num_slots = _table_size * sparsity; + for (size_t slot = 0; slot < num_slots; ++slot) { + if (has_slot(slot)) { + size_t index = (size_t)index_array[slot]; ++count; - size_t ideal_index = get_hash(_table[i]._key); - 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) { + if (index >= _num_entries) { util_cat.error() - << "SimpleHashMap is invalid: key " << _table[i]._key - << " should be in slot " << wants_index << " instead of " - << i << " (ideal is " << ideal_index << ")\n"; + << "SimpleHashMap " << this << " is invalid: slot " << slot + << " contains index " << index << " which is past the end of the" + " 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 +INLINE size_t SimpleHashMap:: +next_hash(size_t hash) const { + return (hash + 1) & ((_table_size * sparsity) - 1); +} + +/** + * Finds the slot in which the given key should fit. + */ +template +INLINE int SimpleHashMap:: +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 +INLINE bool SimpleHashMap:: +has_slot(size_t slot) const { + return get_index_array()[slot] >= 0; } /** @@ -458,40 +533,33 @@ get_hash(const Key &key) const { template INLINE bool SimpleHashMap:: 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 - * value. + * Constructs a new TableEntry with the given slot, storing the indicated key + * and value. */ template -INLINE void SimpleHashMap:: -store_new_element(size_t n, const Key &key, const Value &data) { - new(&_table[n]) TableEntry(key, data); - get_exists_array()[n] = true; +INLINE size_t SimpleHashMap:: +store_new_element(size_t slot, const Key &key, const Value &data) { + size_t index = _num_entries++; + 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. - */ -template -INLINE void SimpleHashMap:: -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) + * 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. * within the table. */ template -INLINE unsigned char *SimpleHashMap:: -get_exists_array() const { - return (unsigned char *)(_table + _table_size); +INLINE int *SimpleHashMap:: +get_index_array() const { + 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). - size_t alloc_size = _table_size * sizeof(TableEntry) + _table_size; + // _table_size * 4 more ints at the end (for the index array). + 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 INLINE bool SimpleHashMap:: 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 INLINE bool SimpleHashMap:: 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. - if (_table_size <= 8 || (_table_size >> 3) < _num_entries) { + // know it's probably time to shrink it down. + 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 void SimpleHashMap:: resize_table(size_t new_size) { nassertv(_table_size != 0); + nassertv(new_size >= _num_entries); - SimpleHashMap old_map(_comp); - swap(old_map); + DeletedBufferChain *old_chain = _deleted_chain; + 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). - size_t alloc_size = _table_size * sizeof(TableEntry) + _table_size; + // _table_size * sparsity more ints at the end (for the sparse index array). + 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(); - memset(exists_array, 0, _table_size); - nassertv(_num_entries == 0); + int *index_array = get_index_array(); + memset(index_array, -1, _table_size * sizeof(int) * sparsity); - // Now copy the entries from the old table into the new table. - for (size_t i = 0; i < old_table_size; ++i) { - if (old_map.has_element(i)) { - size_t new_index = get_hash(old_map._table[i]._key); + // Now copy the entries from the old table into the new table. We don't + // have to reorder these, fortunately. Hopefully, a smart compiler will + // optimize this to a memcpy. + 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) { - // Hash conflict; look for a better spot. This has to succeed. - new_index = (new_index + 1) & (_table_size - 1); - } + // We don't need this old thing anymore. + old_chain->deallocate(old_table, TypeHandle::none()); -#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; + // 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. + slot = next_hash(slot); } + index_array[slot] = (int)i; } nassertv(validate()); - nassertv(old_map.validate()); - - nassertv(_num_entries == old_map._num_entries); } diff --git a/panda/src/putil/simpleHashMap.h b/panda/src/putil/simpleHashMap.h index 928b704e05..d7d6aaa7fc 100644 --- a/panda/src/putil/simpleHashMap.h +++ b/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 - * has a simpler interface (we don't mess around with iterators), (b) it wants - * an additional method on the Compare object, Compare::is_equal(a, b), and - * (c) it doesn't depend on the system STL providing hash_map. + * implemented as a hashtable. It is similar to STL's hash_map, but + * (a) it has a simpler interface (we don't mess around with iterators), + * (b) it wants an additional method on the Compare object, + 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 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 void clear_element(size_t n); - INLINE unsigned char *get_exists_array() const; + INLINE size_t store_new_element(size_t n, const Key &key, const Value &data); + INLINE int *get_index_array() const; void new_table(); INLINE bool consider_expand_table();