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

Replace boost hash usage with TfHash in pxr/base/vt #2176

Merged
merged 1 commit into from
Apr 3, 2023
Merged
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
15 changes: 9 additions & 6 deletions pxr/base/vt/array.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
#include "pxr/base/tf/diagnostic.h"
#include "pxr/base/tf/mallocTag.h"

#include <boost/functional/hash.hpp>
#include <boost/iterator_adaptors.hpp>
#include <boost/iterator/reverse_iterator.hpp>

Expand Down Expand Up @@ -923,14 +922,18 @@ class VtArray : public Vt_ArrayBase {
extern template class VtArray< VT_TYPE(elem) >;
BOOST_PP_SEQ_FOR_EACH(VT_ARRAY_EXTERN_TMPL, ~, VT_SCALAR_VALUE_TYPES)

template <class HashState, class ELEM>
inline std::enable_if_t<VtIsHashable<ELEM>()>
TfHashAppend(HashState &h, VtArray<ELEM> const &array)
{
h.Append(array.size());
h.AppendContiguous(array.cdata(), array.size());
}

template <class ELEM>
typename std::enable_if<VtIsHashable<ELEM>(), size_t>::type
hash_value(VtArray<ELEM> const &array) {
size_t h = array.size();
for (auto const &x: array) {
boost::hash_combine(h, x);
}
return h;
return TfHash()(array);
}

// Specialize traits so others can figure out that VtArray is an array.
Expand Down
3 changes: 1 addition & 2 deletions pxr/base/vt/dictionary.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
#include "pxr/base/tf/hash.h"
#include "pxr/base/tf/mallocTag.h"

#include <boost/functional/hash.hpp>
#include <boost/iterator/iterator_adaptor.hpp>

#include <initializer_list>
Expand Down Expand Up @@ -273,7 +272,7 @@ class VtDictionary {
if (dict.empty())
return 0;
// Otherwise hash the map.
return boost::hash_value(*dict._dictMap);
return TfHash()(*dict._dictMap);
}

/// Inserts a range into the \p VtDictionary.
Expand Down
4 changes: 2 additions & 2 deletions pxr/base/vt/hash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ void
_IssueUnimplementedHashError(std::type_info const &t)
{
TF_CODING_ERROR("Invoked VtHashValue on an object of type <%s>, which "
"is not hashable by boost::hash<>() or TfHash(). Consider "
"providing an overload of hash_value().",
"is not hashable by TfHash(). Consider "
"providing an overload of hash_value() or TfHashAppend().",
ArchGetDemangled(t).c_str());
}

Expand Down
26 changes: 5 additions & 21 deletions pxr/base/vt/hash.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
#include "pxr/pxr.h"
#include "pxr/base/vt/api.h"
#include "pxr/base/tf/hash.h"
#include <boost/functional/hash.hpp>
#include <typeinfo>
#include <utility>

Expand All @@ -38,28 +37,15 @@ namespace Vt_HashDetail {
// Issue a coding error when we attempt to hash a t.
VT_API void _IssueUnimplementedHashError(std::type_info const &t);

// We make unqualified calls, intending to pick up boost::hash_value if an
// overload isn't found by ADL.
using boost::hash_value;

// A constexpr function that determines hashability.
template <class T, class = decltype(hash_value(std::declval<T>()))>
constexpr bool _IsHashable(int) { return true; }
template <class T, class = decltype(TfHash()(std::declval<T>()))>
constexpr bool _IsHashable(long) { return true; }
template <class T>
constexpr bool _IsHashable(...) { return false; }

// Hash implementations -- We're using an overload resolution ordering trick
// here (int vs long vs ...) so that we pick hash_value first, if possible,
// otherwise we do TfHash() if possible, otherwise we issue a runtime error.
template <class T, class = decltype(hash_value(std::declval<T>()))>
inline size_t
_HashValueImpl(T const &val, int)
{
return hash_value(val);
}

// here (long vs ...) so that we pick TfHash() if possible, otherwise
// we issue a runtime error.
template <class T, class = decltype(TfHash()(std::declval<T>()))>
inline size_t
_HashValueImpl(T const &val, long)
Expand All @@ -79,17 +65,15 @@ _HashValueImpl(T const &val, ...)


/// A constexpr function that returns true if T is hashable via VtHashValue,
/// false otherwise. This is true if we can either invoke
/// (boost::)hash_value() or TfHash()() on a T instance.
/// false otherwise. This is true if we can invoke TfHash()() on a T instance.
template <class T>
constexpr bool
VtIsHashable() {
return Vt_HashDetail::_IsHashable<T>(0);
}

/// Compute a hash code for \p val by invoking (boost::)hash_value(val) if
/// possible, otherwise by invoking TfHash()(val), or if neither are possible
/// issue a coding error and return 0.
/// Compute a hash code for \p val by invoking TfHash()(val) or when not
/// possible issue a coding error and return 0.
template <class T>
size_t VtHashValue(T const &val)
{
Expand Down
12 changes: 12 additions & 0 deletions pxr/base/vt/testenv/testVtCpp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1549,6 +1549,9 @@ static void
testValueHash()
{
static_assert(VtIsHashable<int>(), "");
static_assert(VtIsHashable<double>(), "");
static_assert(VtIsHashable<GfVec3f>(), "");
static_assert(VtIsHashable<std::string>(), "");
static_assert(!VtIsHashable<_Unhashable>(), "");

VtValue vHashable{1};
Expand All @@ -1574,6 +1577,14 @@ testValueHash()
}
}

static void
testArrayHash()
{
VtArray<int> array = {1, 2, 3, 4, 5, 10, 100};
TF_AXIOM(TfHash()(array) == TfHash()(array));
TF_AXIOM(TfHash()(array) == TfHash()(VtArray<int>(array)));
}

template <class T>
struct _TypedProxy : VtTypedValueProxyBase
{
Expand Down Expand Up @@ -1849,6 +1860,7 @@ int main(int argc, char *argv[])

testValue();
testValueHash();
testArrayHash();
testTypedVtValueProxy();
testErasedVtValueProxy();
testCombinedVtValueProxies();
Expand Down
12 changes: 1 addition & 11 deletions pxr/base/vt/value.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -309,20 +309,10 @@ class Vt_CastRegistry {
using _ConversionSourceToTarget =
std::pair<std::type_index, std::type_index>;

struct _ConversionSourceToTargetHash
{
std::size_t operator()(_ConversionSourceToTarget p) const
{
std::size_t h = p.first.hash_code();
boost::hash_combine(h, p.second.hash_code());
return h;
}
};

using _Conversions = tbb::concurrent_unordered_map<
_ConversionSourceToTarget,
VtValue (*)(VtValue const &),
_ConversionSourceToTargetHash>;
TfHash>;

_Conversions _conversions;

Expand Down