Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: Replace robin-hood-hashing with unordered_dense #7558

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions .github/workflows/vvl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,30 +60,30 @@ jobs:

linux:
runs-on: ubuntu-22.04
name: "linux (${{matrix.sanitize}} sanitizer, ${{matrix.config}}, robinhood ${{matrix.robin_hood}} )"
name: "linux (${{matrix.sanitize}} sanitizer, ${{matrix.config}}, unordered_dense ${{matrix.unordered_dense}} )"
strategy:
fail-fast: false
matrix:
sanitize: [ address, thread ]
config: [debug, release]
robin_hood: [ "ON" ]
unordered_dense: [ "ON" ]
include:
# Test with Robin Hood disabled
# Test with unordered_dense disabled
# Chromium build, and some package managers don't use it.
- config: release
robin_hood: "OFF"
unordered_dense: "OFF"
sanitize: address
steps:
- uses: actions/checkout@v4
- uses: lukka/get-cmake@latest
- uses: hendrikmuhs/[email protected]
with:
key: ${{ matrix.config }}-${{ matrix.sanitize }}-${{matrix.robin_hood}}
key: ${{ matrix.config }}-${{ matrix.sanitize }}-${{matrix.unordered_dense}}
- run: sudo apt-get -qq update && sudo apt-get install -y libwayland-dev xorg-dev
# This is to combat a bug when using 6.6 linux kernels with thread/address sanitizer
# https://github.com/google/sanitizers/issues/1716
- run: sudo sysctl vm.mmap_rnd_bits=28
- run: python scripts/tests.py --build --config ${{ matrix.config }} --cmake='-DUSE_ROBIN_HOOD_HASHING=${{matrix.robin_hood}}'
- run: python scripts/tests.py --build --config ${{ matrix.config }} --cmake='-DUSE_CUSTOM_HASH_MAP=${{matrix.unordered_dense}}'
env:
CFLAGS: -fsanitize=${{ matrix.sanitize }}
CXXFLAGS: -fsanitize=${{ matrix.sanitize }}
Expand Down Expand Up @@ -148,7 +148,7 @@ jobs:
build-ci/external/glslang/build/install
build-ci/external/googltest/build/install
build-ci/external/mimalloc/build/install
build-ci/external/robin-hood-hashing/build/install
build-ci/external/unordered_dense/build/install
build-ci/external/SPIRV-Headers/build/install
build-ci/external/SPIRV-Tools/build/install
build-ci/external/Vulkan-Headers/build/install
Expand Down
12 changes: 8 additions & 4 deletions layers/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,15 @@ target_link_libraries(VkLayer_utils PUBLIC
target_include_directories(VkLayer_utils SYSTEM PRIVATE external)
target_include_directories(VkLayer_utils PUBLIC . ${API_TYPE})

find_package(robin_hood CONFIG)
option(USE_ROBIN_HOOD_HASHING "robin_hood provides faster versions of std::unordered_map and std::unordered_set" ${robin_hood_FOUND})
if (USE_ROBIN_HOOD_HASHING)
target_link_libraries(VkLayer_utils PUBLIC robin_hood::robin_hood)
target_compile_definitions(VkLayer_utils PUBLIC USE_ROBIN_HOOD_HASHING)
message(WARNING "Option USE_ROBIN_HOOD_HASHING has been deprecated. Use USE_CUSTOM_HASH_MAP instead.")
endif ()

find_package(unordered_dense CONFIG)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switching from USE_ROBIN_...* to USE_UNORDERED_* is going to require changes in Internal CI and the SDK build stuff. I wonder if we should do something like USE_CUSTOM_HASHMAP instead in case we ever decide to change implementations again. Curious what others think about this...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea I think. I only used this name in replacement of the C++ define, explicit mentions of unordered dense are still there in files like vvl.yml

option(USE_CUSTOM_HASH_MAP "Use a custom hash map (unordered_dense). It provides faster versions of std::unordered_map and std::unordered_set" ${unordered_dense_FOUND})
if (USE_CUSTOM_HASH_MAP)
target_link_libraries(VkLayer_utils PUBLIC unordered_dense::unordered_dense)
target_compile_definitions(VkLayer_utils PUBLIC USE_CUSTOM_HASH_MAP)
endif()

# Using mimalloc on non-Windows OSes currently results in unit test instability with some
Expand Down
64 changes: 14 additions & 50 deletions layers/containers/custom_containers.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,65 +32,32 @@
#include <optional>
#include <utility>

#ifdef USE_ROBIN_HOOD_HASHING
#include "robin_hood.h"
#ifdef USE_CUSTOM_HASH_MAP
#include "ankerl/unordered_dense.h"
#else
#include <unordered_set>
#endif

// namespace aliases to allow map and set implementations to easily be swapped out
namespace vvl {

#ifdef USE_ROBIN_HOOD_HASHING
#ifdef USE_CUSTOM_HASH_MAP
template <typename T>
using hash = robin_hood::hash<T>;
struct hash {
std::size_t operator()(const T &s) const noexcept { return static_cast<std::size_t>(internal_hash{}(s)); }

template <typename Key, typename Hash = robin_hood::hash<Key>, typename KeyEqual = std::equal_to<Key>>
using unordered_set = robin_hood::unordered_set<Key, Hash, KeyEqual>;

template <typename Key, typename T, typename Hash = robin_hood::hash<Key>, typename KeyEqual = std::equal_to<Key>>
using unordered_map = robin_hood::unordered_map<Key, T, Hash, KeyEqual>;

template <typename Key, typename T>
using map_entry = robin_hood::pair<Key, T>;

// robin_hood-compatible insert_iterator (std:: uses the wrong insert method)
// NOTE: std::iterator was deprecated in C++17, and newer versions of libstdc++ appear to mark this as such.
template <typename T>
struct insert_iterator {
using iterator_category = std::output_iterator_tag;
using value_type = typename T::value_type;
using iterator = typename T::iterator;
using difference_type = void;
using pointer = void;
using reference = T &;

insert_iterator(reference t, iterator i) : container(&t), iter(i) {}

insert_iterator &operator=(const value_type &value) {
auto result = container->insert(value);
iter = result.first;
++iter;
return *this;
}

insert_iterator &operator=(value_type &&value) {
auto result = container->insert(std::move(value));
iter = result.first;
++iter;
return *this;
}

insert_iterator &operator*() { return *this; }
private:
using internal_hash = ankerl::unordered_dense::hash<T>;
};

insert_iterator &operator++() { return *this; }
template <typename Key, typename Hash = ankerl::unordered_dense::hash<Key>, typename KeyEqual = std::equal_to<Key>>
using unordered_set = ankerl::unordered_dense::set<Key, Hash, KeyEqual>;

insert_iterator &operator++(int) { return *this; }
template <typename Key, typename T, typename Hash = ankerl::unordered_dense::hash<Key>, typename KeyEqual = std::equal_to<Key>>
using unordered_map = ankerl::unordered_dense::segmented_map<Key, T, Hash, KeyEqual>;

private:
T *container;
iterator iter;
};
template <typename Key, typename T>
using map_entry = std::pair<Key, T>;
#else
template <typename T>
using hash = std::hash<T>;
Expand All @@ -103,9 +70,6 @@ using unordered_map = std::unordered_map<Key, T, Hash, KeyEqual>;

template <typename Key, typename T>
using map_entry = std::pair<Key, T>;

template <typename T>
using insert_iterator = std::insert_iterator<T>;
#endif

} // namespace vvl
Expand Down
7 changes: 2 additions & 5 deletions layers/core_checks/cc_image_layout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,8 @@ void CoreChecks::TransitionFinalSubpassLayouts(vvl::CommandBuffer &cb_state) {

static GlobalImageLayoutRangeMap *GetLayoutRangeMap(GlobalImageLayoutMap &map, const vvl::Image &image_state) {
// This approach allows for a single hash lookup or/create new
auto &layout_map = map[&image_state];
if (!layout_map) {
layout_map.emplace(image_state.subresource_encoder.SubresourceCount());
}
return &(*layout_map);
auto result = map.emplace(&image_state, image_state.subresource_encoder.SubresourceCount());
return &(result.first->second.value());
}

// Helper to update the Global or Overlay layout map
Expand Down
2 changes: 1 addition & 1 deletion layers/state_tracker/image_state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,7 @@ std::vector<VkPresentModeKHR> Surface::GetPresentModes(VkPhysicalDevice phys_dev
assert(phys_dev);
std::vector<VkPresentModeKHR> result;
if (auto search = present_modes_data_.find(phys_dev); search != present_modes_data_.end()) {
for (auto mode = search->second.begin(); mode != search->second.end(); mode++) {
for (auto mode = search->second.begin(); mode != search->second.end(); ++mode) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jeremyg-lunarg so this is the type of issue failing the build before 😆

result.push_back(mode->first);
}
return result;
Expand Down
2 changes: 1 addition & 1 deletion layers/utils/vk_layer_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ class vl_concurrent_unordered_map {
private:
static const int BUCKETS = (1 << BUCKETSLOG2);

vvl::unordered_map<Key, T, Hash> maps[BUCKETS];
std::array<vvl::unordered_map<Key, T, Hash>, BUCKETS> maps;
struct alignas(get_hardware_destructive_interference_size()) AlignedSharedMutex {
std::shared_mutex lock;
};
Expand Down
8 changes: 4 additions & 4 deletions scripts/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# ~~~
# Copyright (c) 2023 LunarG, Inc.
# Copyright (c) 2023-2024 LunarG, Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -122,9 +122,9 @@ if (UPDATE_DEPS)
include("${UPDATE_DEPS_DIR}/helper.cmake")
endif()
endif()
if (ROBIN_HOOD_HASHING_INSTALL_DIR)
list(APPEND CMAKE_PREFIX_PATH ${ROBIN_HOOD_HASHING_INSTALL_DIR})
set(CMAKE_REQUIRE_FIND_PACKAGE_robin_hood TRUE PARENT_SCOPE)
if (UNORDERED_DENSE_INSTALL_DIR)
list(APPEND CMAKE_PREFIX_PATH ${UNORDERED_DENSE_INSTALL_DIR})
set(CMAKE_REQUIRE_FIND_PACKAGE_unordered_dense TRUE PARENT_SCOPE)
endif()
if (GLSLANG_INSTALL_DIR)
list(APPEND CMAKE_PREFIX_PATH ${GLSLANG_INSTALL_DIR})
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (c) 2019-2023 LunarG, Inc.
# Copyright (c) 2019-2024 LunarG, Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand All @@ -18,7 +18,7 @@ vulkan_utility_libraries_dir = "//external/Vulkan-Utility-Libraries"
vvl_spirv_tools_dir = "//external/SPIRV-Tools"
vvl_spirv_headers_dir = "//external/SPIRV-Headers"
vvl_glslang_dir = "//external/glslang/"
robin_hood_headers_dir = "//external/robin-hood-hashing/src/include"
unordered_dense_headers_dir = "//external/unordered_dense/include"

# Subdirectories for generated files
vulkan_data_subdir = ""
Expand Down
17 changes: 7 additions & 10 deletions scripts/known_good.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,12 @@
"commit": "04896c462d9f3f504c99a4698605b6524af813c1"
},
{
"name": "robin-hood-hashing",
"url": "https://github.com/martinus/robin-hood-hashing.git",
"sub_dir": "robin-hood-hashing",
"build_dir": "robin-hood-hashing/build",
"install_dir": "robin-hood-hashing/build/install",
"cmake_options": [
"-DRH_STANDALONE_PROJECT=OFF"
],
"commit": "3.11.5"
"name": "unordered_dense",
"url": "https://github.com/martinus/unordered_dense.git",
"sub_dir": "unordered_dense",
"build_dir": "unordered_dense/build",
"install_dir": "unordered_dense/build/install",
"commit": "v4.0.4"
spencer-lunarg marked this conversation as resolved.
Show resolved Hide resolved
},
{
"name": "mimalloc",
Expand Down Expand Up @@ -149,7 +146,7 @@
"Vulkan-Utility-Libraries": "VULKAN_UTILITY_LIBRARIES_INSTALL_DIR",
"SPIRV-Headers": "SPIRV_HEADERS_INSTALL_DIR",
"SPIRV-Tools": "SPIRV_TOOLS_INSTALL_DIR",
"robin-hood-hashing": "ROBIN_HOOD_HASHING_INSTALL_DIR",
"unordered_dense": "UNORDERED_DENSE_INSTALL_DIR",
"googletest": "GOOGLETEST_INSTALL_DIR",
"mimalloc": "MIMALLOC_INSTALL_DIR"
}
Expand Down