From 2d6b43827ef90d5cd9905146dfa672ff90ff33d3 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 12 Dec 2018 13:47:28 -0800 Subject: [PATCH] Improve StaticString hashing In a lot of case the StaticString hash can be computed a compile time, so we now take advantage of that. Removed StaticString(const char*) ctor, and replaced it with a StaticString::make() method. Fixed a couple wrong uses of the old StaticString(const char*) ctor. --- filament/src/DebugRegistry.cpp | 2 +- filament/src/driver/opengl/OpenGLDriver.cpp | 2 +- filament/src/driver/vulkan/VulkanDriver.cpp | 2 +- .../private/filament/SamplerInterfaceBlock.h | 6 +- .../private/filament/UniformInterfaceBlock.h | 6 +- libs/image/src/ImageSampler.cpp | 4 +- libs/utils/include/utils/CString.h | 70 +++++++++++++------ libs/utils/test/test_CString.cpp | 15 ++++ 8 files changed, 76 insertions(+), 31 deletions(-) diff --git a/filament/src/DebugRegistry.cpp b/filament/src/DebugRegistry.cpp index 70aa22439b..32d3fec672 100644 --- a/filament/src/DebugRegistry.cpp +++ b/filament/src/DebugRegistry.cpp @@ -34,7 +34,7 @@ FDebugRegistry::FDebugRegistry() noexcept { UTILS_NOINLINE void *FDebugRegistry::getPropertyAddress(const char *name) noexcept { - StaticString key(name, strlen(name)); + StaticString key = StaticString::make(name, strlen(name)); auto &propertyMap = mPropertyMap; if (propertyMap.find(key) == propertyMap.end()) { return nullptr; diff --git a/filament/src/driver/opengl/OpenGLDriver.cpp b/filament/src/driver/opengl/OpenGLDriver.cpp index 593811a706..707cbb7da8 100644 --- a/filament/src/driver/opengl/OpenGLDriver.cpp +++ b/filament/src/driver/opengl/OpenGLDriver.cpp @@ -165,7 +165,7 @@ OpenGLDriver::OpenGLDriver(OpenGLPlatform* platform) noexcept ExtentionSet exts; for (GLint i = 0; i < n; i++) { const char * const ext = (const char*)glGetStringi(GL_EXTENSIONS, (GLuint)i); - exts.emplace(ext, strlen(ext)); + exts.insert(StaticString::make(ext, strlen(ext))); if (DEBUG_PRINT_EXTENSIONS) { slog.d << ext << io::endl; } diff --git a/filament/src/driver/vulkan/VulkanDriver.cpp b/filament/src/driver/vulkan/VulkanDriver.cpp index a57d567198..e324a9d269 100644 --- a/filament/src/driver/vulkan/VulkanDriver.cpp +++ b/filament/src/driver/vulkan/VulkanDriver.cpp @@ -948,7 +948,7 @@ void VulkanDriver::debugCommand(const char* methodName) { static const utils::StaticString BEGIN_COMMAND = "beginRenderPass"; static const utils::StaticString END_COMMAND = "endRenderPass"; static bool inRenderPass = false; - const utils::StaticString command(methodName, strlen(methodName)); + const utils::StaticString command = utils::StaticString::make(methodName, strlen(methodName)); if (command == BEGIN_COMMAND) { assert(!inRenderPass); inRenderPass = true; diff --git a/libs/filabridge/include/private/filament/SamplerInterfaceBlock.h b/libs/filabridge/include/private/filament/SamplerInterfaceBlock.h index 39ac2c5e4a..fef3274181 100644 --- a/libs/filabridge/include/private/filament/SamplerInterfaceBlock.h +++ b/libs/filabridge/include/private/filament/SamplerInterfaceBlock.h @@ -55,9 +55,10 @@ public: Builder& name(utils::CString const& interfaceBlockName); Builder& name(utils::CString&& interfaceBlockName); Builder& name(utils::StaticString const& interfaceBlockName); + template Builder& name(utils::StringLiteral const& interfaceBlockName) { - return name(utils::StaticString{interfaceBlockName, N - 1}); + return name(utils::StaticString{ interfaceBlockName }); } // Add a sampler @@ -70,10 +71,11 @@ public: Builder& add(utils::StaticString const& samplerName, Type type, Format format, Precision precision = Precision::MEDIUM, bool multisample = false) noexcept; + template Builder& add(utils::StringLiteral const& samplerName, Type type, Format format, Precision precision = Precision::MEDIUM, bool multisample = false) { - return add(utils::StaticString{ samplerName, N - 1 }, type, format, precision); + return add(utils::StaticString{ samplerName }, type, format, precision); } // build and return the SamplerInterfaceBlock diff --git a/libs/filabridge/include/private/filament/UniformInterfaceBlock.h b/libs/filabridge/include/private/filament/UniformInterfaceBlock.h index f9b3a057ed..e82459f1bb 100644 --- a/libs/filabridge/include/private/filament/UniformInterfaceBlock.h +++ b/libs/filabridge/include/private/filament/UniformInterfaceBlock.h @@ -51,9 +51,10 @@ public: Builder& name(utils::CString const& interfaceBlockName); Builder& name(utils::CString&& interfaceBlockName); Builder& name(utils::StaticString const& interfaceBlockName); + template Builder& name(utils::StringLiteral const& interfaceBlockName) { - return name(utils::StaticString{ interfaceBlockName, (utils::CString::size_type)(N - 1) }); + return name(utils::StaticString{ interfaceBlockName }); } // Add a uniform @@ -63,10 +64,11 @@ public: Type type, Precision precision = Precision::DEFAULT); Builder& add(utils::StaticString const& uniformName, size_t size, Type type, Precision precision = Precision::DEFAULT); + template Builder& add(utils::StringLiteral const& uniformName, size_t size, Type type, Precision precision = Precision::DEFAULT) { - return add(utils::StaticString{ uniformName, N - 1 }, size, type, precision); + return add(utils::StaticString{ uniformName }, size, type, precision); } // build and return the UniformInterfaceBlock diff --git a/libs/image/src/ImageSampler.cpp b/libs/image/src/ImageSampler.cpp index ab6fd4ebba..db6f0d1f63 100644 --- a/libs/image/src/ImageSampler.cpp +++ b/libs/image/src/ImageSampler.cpp @@ -361,8 +361,8 @@ Filter filterFromString(const char* rawname) { { "MINIMUM", Filter::MINIMUM}, }; string name = rawname; - for (auto& c: name) c = toupper((unsigned char) c); - auto iter = map.find({ name.c_str(), name.size() }); + for (auto& c: name) { c = toupper((unsigned char)c); } + auto iter = map.find(StaticString::make(name.c_str(), name.size())); return iter == map.end() ? Filter::DEFAULT : iter->second; } diff --git a/libs/utils/include/utils/CString.h b/libs/utils/include/utils/CString.h index 869866bda2..934820b572 100644 --- a/libs/utils/include/utils/CString.h +++ b/libs/utils/include/utils/CString.h @@ -27,6 +27,19 @@ namespace utils { +//! \privatesection +struct hashCStrings { + typedef const char* argument_type; + typedef size_t result_type; + result_type operator()(argument_type cstr) const noexcept { + size_t hash = 5381; + while (int c = *cstr++) { + hash = (hash * 33u) ^ size_t(c); + } + return hash; + } +}; + //! \privatesection struct equalCStrings { typedef const char* first_argument_type; @@ -37,20 +50,6 @@ struct equalCStrings { } }; -//! \privatesection -struct hashCStrings { - typedef const char* argument_type; - typedef size_t result_type; - result_type operator()(argument_type cstr) const noexcept { - // TODO: is this good enough? - size_t hash = 5381; - while (int c = *cstr++) { - hash = hash * 33 ^ c; - } - return hash; - } -}; - //! \privatesection struct lessCStrings { typedef const char* first_argument_type; @@ -80,24 +79,40 @@ public: StaticString() noexcept = default; + // initialization from a string literal template StaticString(StringLiteral const& other) noexcept // NOLINT(google-explicit-constructor) : mString(other), - mLength(size_type(N - 1)) { - } - - StaticString(const_pointer literal, size_t length) noexcept - : mString(literal), - mLength(size_type(length)) { + mLength(size_type(N - 1)), + mHash(computeHash(other)) { } + // assignment from a string literal template StaticString& operator=(StringLiteral const& other) noexcept { mString = other; mLength = size_type(N - 1); + mHash = computeHash(other); return *this; } + // helper to make a StaticString from a C string that is known to be a string literal + static constexpr StaticString make(const_pointer literal, size_t length) noexcept { + StaticString r; + r.mString = literal; + r.mLength = size_type(length); + size_type hash = 5381; + while (int c = *literal++) { + hash = (hash * 33u) ^ size_type(c); + } + r.mHash = hash; + return r; + } + + static StaticString make(const_pointer literal) noexcept { + return make(literal, strlen(literal)); + } + const_pointer c_str() const noexcept { return mString; } const_pointer data() const noexcept { return mString; } size_type size() const noexcept { return mLength; } @@ -130,9 +145,21 @@ public: return begin()[size() - 1]; } + size_type getHash() const noexcept { return mHash; } + private: const_pointer mString = nullptr; size_type mLength = 0; + size_type mHash = 0; + + template + static constexpr size_type computeHash(StringLiteral const& s) noexcept { + size_type hash = 5381; + for (size_t i = 0; i < N - 1; i++) { + hash = (hash * 33u) ^ size_type(s[i]); + } + return hash; + } int compare(const StaticString& rhs) const noexcept; @@ -344,9 +371,8 @@ template<> struct hash { typedef utils::StaticString argument_type; typedef size_t result_type; - utils::hashCStrings hasher; size_t operator()(const utils::StaticString& s) const noexcept { - return hasher(s.c_str()); + return s.getHash(); } }; diff --git a/libs/utils/test/test_CString.cpp b/libs/utils/test/test_CString.cpp index 5e1a0e52b0..6c9545e3a1 100644 --- a/libs/utils/test/test_CString.cpp +++ b/libs/utils/test/test_CString.cpp @@ -24,3 +24,18 @@ TEST(CString, EmptyString) { CString emptyString(""); EXPECT_STREQ("", emptyString.c_str_safe()); } + +TEST(StaticString, hash) { + StaticString a("Hello World!"); + StaticString b = StaticString::make("Hello World!"); + StaticString c("Hello World"); + StaticString d("Hello World!"); + + EXPECT_EQ(a.getHash(), b.getHash()); + EXPECT_EQ(a.getHash(), d.getHash()); + EXPECT_NE(a.getHash(), c.getHash()); + EXPECT_NE(b.getHash(), c.getHash()); + + std::hash ha; + EXPECT_EQ(ha(a), a.getHash()); +}