Skip to content

Commit

Permalink
Swap to boost::unordered_flat_map
Browse files Browse the repository at this point in the history
The performance of `std::unordered_map` is widely known to be
sub-optimal due to the requirement for iterator and reference stability.
This precludes

An up-to-date benchmark can be found here:
https://jacksonallan.github.io/c_cpp_hash_tables_benchmark, which shows
that `boost::unordered_flat_map` performs extremely well for inserts and
lookups, which is all we do.  The lack of iteration speed, the erasure
residual performance impact, and the minimum capacity of 30 all have no
affect on us.

The `boost::unordered_flat_map` implementation has been taken from
https://github.com/MikePopoloski/boost_unordered.

Add more implementation to `fixed_string` as we now need to be
move-constructible and move-assignable since we will be moved around
inside `boost::unordered_flat_map`.

Add `SYSTEM` to the include path for `thirdparty` so we don't get
warnings on external code.
  • Loading branch information
elliotgoodrich committed Oct 17, 2024
1 parent b34f74d commit b1ecb4c
Show file tree
Hide file tree
Showing 7 changed files with 10,274 additions and 13 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ add_executable(

target_compile_definitions(trimja PRIVATE TRIMJA_VERSION="${CMAKE_PROJECT_VERSION}")
target_include_directories(trimja PRIVATE src)
target_include_directories(trimja PRIVATE thirdparty)
target_include_directories(trimja SYSTEM PRIVATE thirdparty)
target_compile_options(trimja PRIVATE
$<$<NOT:$<CXX_COMPILER_ID:MSVC>>:-Wall -Wextra -Wpedantic>
$<$<CXX_COMPILER_ID:MSVC>:/W4>
Expand Down
5 changes: 3 additions & 2 deletions src/basicscope.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@

#include "fixed_string.h"

#include <boost/boost_unordered.hpp>

#include <string>
#include <string_view>
#include <unordered_map>

namespace trimja {

Expand All @@ -36,7 +37,7 @@ namespace trimja {
* @brief Manages a scope of variables for evaluation and substitution.
*/
class BasicScope {
std::unordered_map<fixed_string, std::string> m_variables;
boost::unordered_flat_map<fixed_string, std::string> m_variables;

public:
/**
Expand Down
23 changes: 23 additions & 0 deletions src/fixed_string.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,16 @@ fixed_string::~fixed_string() {
delete[] m_data.data();
}

fixed_string::fixed_string(fixed_string&& other) noexcept
: m_data(std::exchange(other.m_data, std::string_view{nullptr, 0})) {}

fixed_string& fixed_string::operator=(fixed_string&& rhs) noexcept {
fixed_string tmp{std::move(rhs)};
using std::swap;
swap(m_data, tmp.m_data);
return *this;
}

fixed_string::operator std::string_view() const {
return m_data;
}
Expand All @@ -67,4 +77,17 @@ bool operator!=(const fixed_string& lhs, const fixed_string& rhs) {
return lhs.view() != rhs.view();
}

std::size_t hash_value(const fixed_string& str) {
return std::hash<fixed_string>{}(str);
}

} // namespace trimja

namespace std {

size_t hash<trimja::fixed_string>::operator()(
const trimja::fixed_string& str) const {
return hash<std::string_view>{}(str.view());
}

} // namespace std
34 changes: 28 additions & 6 deletions src/fixed_string.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ namespace trimja {
* changed after construction.
*
* This class is useful as the key to an associative container since these are
* immutable after construction. It has no small-string optimization (yet).
* immutable after construction. It has no small-string optimization.
*/
class fixed_string {
std::string_view m_data;
Expand Down Expand Up @@ -79,8 +79,24 @@ class fixed_string {

~fixed_string();

fixed_string(const fixed_string&) = delete;
fixed_string& operator=(const fixed_string&) = delete;
/**
* @brief Move construct from another fixed_string.
*
* `other` will be left as an empty string.
*
* @param str The fixed_string to move from.
*/
fixed_string(fixed_string&& other) noexcept;

/**
* @brief Move assign from another fixed_string.
*
* `other` will be left as an empty string.
*
* @param str The fixed_string to move assign from.
* @return A reference to this.
*/
fixed_string& operator=(fixed_string&& rhs) noexcept;

/**
* @brief Implicit conversion operator to std::string_view.
Expand Down Expand Up @@ -115,6 +131,14 @@ bool operator==(const fixed_string& lhs, const fixed_string& rhs);
*/
bool operator!=(const fixed_string& lhs, const fixed_string& rhs);

/**
* @brief Computes the hash value for a fixed_string.
*
* @param str The fixed_string to hash.
* @return The hash value of the fixed_string.
*/
std::size_t hash_value(const fixed_string& str);

} // namespace trimja

namespace std {
Expand All @@ -130,9 +154,7 @@ struct hash<trimja::fixed_string> {
* @param str The fixed_string to hash.
* @return The hash value of the fixed_string.
*/
size_t operator()(const trimja::fixed_string& str) const {
return hash<std::string_view>{}(str.view());
}
size_t operator()(const trimja::fixed_string& str) const;
};

} // namespace std
Expand Down
15 changes: 12 additions & 3 deletions src/graph.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,13 @@

#include "fixed_string.h"

#include <boost/boost_unordered.hpp>

#include <numeric>
#include <optional>
#include <set>
#include <string>
#include <string_view>
#include <unordered_map>
#include <vector>

namespace trimja {
Expand All @@ -42,15 +43,22 @@ namespace trimja {
*/
class Graph {
struct PathHash {
// We need `PathHash` to have some size, otherwise we hit a compilation
// issue with `boost::unordered_flat_map`.
void* _;
std::size_t operator()(const fixed_string& v) const;
};

struct PathEqual {
// We need `PathEqual` to have some size, otherwise we hit a compilation
// issue with `boost::unordered_flat_map`.
void* _;
bool operator()(const fixed_string& left, const fixed_string& right) const;
};

private:
// A look up from path to vertex index.
std::unordered_map<fixed_string, std::size_t, PathHash, PathEqual>
boost::unordered_flat_map<fixed_string, std::size_t, PathHash, PathEqual>
m_pathToIndex;

// An adjacency list of input -> output
Expand All @@ -60,7 +68,8 @@ class Graph {
std::vector<std::set<std::size_t>> m_outputToInput;

// Names of paths (this points to the keys in `m_pathToIndex`, which is always
// valid since `std::unordered_map` has pointer stability.
// valid since `fixed_string` has no small-string optimization and always
// allocates on the heap.
std::vector<std::string_view> m_path;

std::size_t m_defaultIndex = std::numeric_limits<std::size_t>::max();
Expand Down
4 changes: 3 additions & 1 deletion src/trimutil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "murmur_hash.h"
#include "rule.h"

#include <boost/boost_unordered.hpp>
#include <ninja/util.h>

#include <cassert>
Expand Down Expand Up @@ -103,7 +104,8 @@ struct BuildContext {
std::vector<std::pair<Rule, std::size_t>> rules;

// Our rules keyed by name
std::unordered_map<fixed_string, std::size_t> ruleLookup;
boost::unordered_flat_map<fixed_string, std::size_t, std::hash<trimja::fixed_string>> ruleLookup;
//hash<trimja::fixed_string>

// Our top-level variables
BasicScope fileScope;
Expand Down
Loading

0 comments on commit b1ecb4c

Please sign in to comment.