Skip to content

Commit

Permalink
Two-level hashing of std::type_info * via pointer & string hash (f…
Browse files Browse the repository at this point in the history
…ixes #283)

Exchange of RTTI type information between separately compiled extension
libraries tends to be fragile: depending on the compiler and platform,
the C++ standard library may use one of two strategies to decide whether
two ``std::type_info`` instances are equal. The first is to perform a
string comparison of the mangled type name. When types are organized in
hash tables, a string hash is then also needed. This strategy yields the
expected result but can be rather inefficient.

Other platforms simply compare the pointer value and rely on a
poiner-based hashing scheme. This is far more efficient but requires
that the linker merges duplicate RTTI information from separate shared
libraries.

Unfortunately, this does not always work for the following reasons:

1. Python passes the ``RTLD_LOCAL``  flag to ``dlopen()`` when it loads
   shared libraries. If RTTI symbols are exported by a separate shared
   library, then things may still be fine. But if the Python extension
   is in charge of exporting RTTI symbols, there is problem.

2. It can generally be tricky to get the compiler to export RTTI symbols
   for non-polymorphic types.

3. Setting the right ``__attribute__ ((visibility("default")))`` flags
   in extension libraries is error-prone and a source of confusion for
   new users.

This commit changes nanobind to adopt both strategies at the same time.
Type queries first go through a fast pointer-based hash table followed
by a secondary string-based hash table. In the latter case, nanobind
also populates the faster pointer-based table with the missing
information so that the fast path eventually resolves all of the
queries.

This commit changes the internal representation of nanobind data
structures, hence the ABI version had to be incremented.
  • Loading branch information
wjakob committed Oct 22, 2023
1 parent 11a9b3c commit b515b1f
Show file tree
Hide file tree
Showing 10 changed files with 279 additions and 153 deletions.
2 changes: 2 additions & 0 deletions cmake/nanobind-config.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,8 @@ function (nanobind_build_library TARGET_NAME)
${NB_DIR}/include/nanobind/eigen/sparse.h

${NB_DIR}/src/buffer.h
${NB_DIR}/src/hash.h
${NB_DIR}/src/hash.cpp
${NB_DIR}/src/nb_internals.h
${NB_DIR}/src/nb_internals.cpp
${NB_DIR}/src/nb_func.cpp
Expand Down
50 changes: 0 additions & 50 deletions docs/faq.rst
Original file line number Diff line number Diff line change
Expand Up @@ -236,56 +236,6 @@ will:
definition is changed, only a subset of the binding code will generally need
to be recompiled.

Nanobind cannot pass instances of my type in a multi-library/extension project
------------------------------------------------------------------------------

Suppose that nanobind unexpectedly raises a ``TypeError`` when passing or
returning an instance of a bound type. There is usually a simple explanation:
the type (let's call it "``Foo``") is defined in a library compiled separately
from the main nanobind extension (let's call it ``libfoo``). The problem can
also arise when there are multiple extension libraries that all make use of
``Foo``.

The problem is that the runtime type information ("RTTI") describing ``Foo`` is
is not synchronized among these different libraries, at which point it appears
to nanobind that there are multiple identically named but distinct types called
``Foo``. The dynamic linker is normally responsible for merging the RTTI
records, but it can only do so when the shared library exports them correctly.

On Windows you must specify a DLL export/import annotation, and on other
platforms it suffices to raise the visibility of the associated symbols.

.. code-block:: cpp
/* TODO: Change 'MYLIB' to the name of your project. It's probably best to put
these into a common header file included by all parts of the project */
#if defined(_WIN32)
# define MYLIB_EXPORT __declspec(dllexport)
# define MYLIB_IMPORT __declspec(dllimport)
#else
# define MYLIB_EXPORT __attribute__ ((visibility("default")))
# define MYLIB_IMPORT __attribute__ ((visibility("default")))
#endif
#if defined(MYLIB_BUILD)
# define MYLIB_API MYLIB_EXPORT
#else
# define MYLIB_API MYLIB_IMPORT
#endif
/// Important: annotate the Class declaration with MYLIB_API
class MYLIB_API Foo {
// ... Foo definitions ..
};
In the CMake build system, you must furthermore specify the ``-DMYLIB_BUILD``
definition so that symbols are exported when building ``libfoo`` and imported
by consumers of ``libfoo``.

.. code-block:: cmake
target_compile_definitions(libfoo PRIVATE MYLIB_BUILD)
.. _type-visibility:

How can I avoid conflicts with other projects using nanobind?
Expand Down
4 changes: 4 additions & 0 deletions include/nanobind/nb_class.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,17 @@ enum class type_init_flags : uint32_t {
all_init_flags = (0x1f << 19)
};

// See internals.h
struct nb_alias_chain;

/// Information about a type that persists throughout its lifetime
struct type_data {
uint32_t size;
uint32_t align : 8;
uint32_t flags : 24;
const char *name;
const std::type_info *type;
nb_alias_chain *alias_chain;
PyTypeObject *type_py;
void (*destruct)(void *);
void (*copy)(void *, const void *);
Expand Down
94 changes: 94 additions & 0 deletions src/hash.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
#include "hash.h"

#if defined(_MSC_VER)
# define ROTL32(x,y) _rotl(x,y)
# define ROTL64(x,y) _rotl64(x,y)

#else
inline uint32_t rotl32(uint32_t x, int8_t r) {
return (x << r) | (x >> (32 - r));
}

inline uint64_t rotl64(uint64_t x, int8_t r) {
return (x << r) | (x >> (64 - r));
}

# define ROTL32(x,y) rotl32(x,y)
# define ROTL64(x,y) rotl64(x,y)
#endif

//-----------------------------------------------------------------------------

uint64_t MurmurHash3_x64_64(const void *key, const size_t len,
const uint32_t seed) {
const uint8_t *data = (const uint8_t *) key;
const size_t nblocks = len / 16;

uint64_t h1 = seed;
uint64_t h2 = seed;

const uint64_t c1 = (uint64_t) 0x87c37b91114253d5ull;
const uint64_t c2 = (uint64_t) 0x4cf5ad432745937full;

//----------
// body

const uint64_t * blocks = (const uint64_t *)(data);

for(size_t i = 0; i < nblocks; i++) {
uint64_t k1 = blocks[i*2+0];
uint64_t k2 = blocks[i*2+1];

k1 *= c1; k1 = ROTL64(k1,31); k1 *= c2; h1 ^= k1;

h1 = ROTL64(h1,27); h1 += h2; h1 = h1*5+0x52dce729;

k2 *= c2; k2 = ROTL64(k2,33); k2 *= c1; h2 ^= k2;

h2 = ROTL64(h2,31); h2 += h1; h2 = h2*5+0x38495ab5;
}

//----------
// tail

const uint8_t *tail = (const uint8_t *) (data + nblocks * 16);

uint64_t k1 = 0;
uint64_t k2 = 0;

switch(len & 15) {
case 15: k2 ^= ((uint64_t)tail[14]) << 48;
case 14: k2 ^= ((uint64_t)tail[13]) << 40;
case 13: k2 ^= ((uint64_t)tail[12]) << 32;
case 12: k2 ^= ((uint64_t)tail[11]) << 24;
case 11: k2 ^= ((uint64_t)tail[10]) << 16;
case 10: k2 ^= ((uint64_t)tail[ 9]) << 8;
case 9: k2 ^= ((uint64_t)tail[ 8]) << 0;
k2 *= c2; k2 = ROTL64(k2,33); k2 *= c1; h2 ^= k2;

case 8: k1 ^= ((uint64_t)tail[ 7]) << 56;
case 7: k1 ^= ((uint64_t)tail[ 6]) << 48;
case 6: k1 ^= ((uint64_t)tail[ 5]) << 40;
case 5: k1 ^= ((uint64_t)tail[ 4]) << 32;
case 4: k1 ^= ((uint64_t)tail[ 3]) << 24;
case 3: k1 ^= ((uint64_t)tail[ 2]) << 16;
case 2: k1 ^= ((uint64_t)tail[ 1]) << 8;
case 1: k1 ^= ((uint64_t)tail[ 0]) << 0;
k1 *= c1; k1 = ROTL64(k1,31); k1 *= c2; h1 ^= k1;
};

//----------
// finalization

h1 ^= len; h2 ^= len;

h1 += h2;
h2 += h1;

h1 = fmix64(h1);
h2 = fmix64(h2);

h1 += h2;

return h1;
}
39 changes: 39 additions & 0 deletions src/hash.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
//-----------------------------------------------------------------------------
// Slightly adapted version of the MurmurHash3 codebase (originally by Austin
// Appleby, in the public domain)
//
// The changes are as follows:
//
// - fmix32 and fmix64 are exported to other compilation units, since they
// are useful has a hash function for 32/64 bit integers and pointers
//
// - The MurmurHash3_x64_64() function is a variant of the original
// MurmurHash3_x64_128() that only returns the low 64 bit of the hash
// value.
//-----------------------------------------------------------------------------

#pragma once

#include <cstdint>
#include <cstdlib>

inline uint32_t fmix32(uint32_t h) {
h ^= h >> 16;
h *= 0x85ebca6b;
h ^= h >> 13;
h *= 0xc2b2ae35;
h ^= h >> 16;

return h;
}

inline uint64_t fmix64(uint64_t k) {
k ^= k >> 33;
k *= (uint64_t) 0xff51afd7ed558ccdull;
k ^= k >> 33;
k *= (uint64_t) 0xc4ceb9fe1a85ec53ull;
k ^= k >> 33;
return k;
}

extern uint64_t MurmurHash3_x64_64(const void *key, size_t len, uint32_t seed);
20 changes: 6 additions & 14 deletions src/implicit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,10 @@ NAMESPACE_BEGIN(detail)

void implicitly_convertible(const std::type_info *src,
const std::type_info *dst) noexcept {
nb_type_map &type_c2p = internals->type_c2p;
type_data *t = nb_type_c2p(internals, dst);
check(t, "nanobind::detail::implicitly_convertible(src=%s, dst=%s): "
"destination type unknown!", type_name(src), type_name(dst));

nb_type_map::iterator it = type_c2p.find(std::type_index(*dst));
check(it != type_c2p.end(),
"nanobind::detail::implicitly_convertible(src=%s, dst=%s): "
"destination type unknown!", type_name(src), type_name(dst));

type_data *t = it->second;
size_t size = 0;

if (t->flags & (uint32_t) type_flags::has_implicit_conversions) {
Expand All @@ -47,14 +43,10 @@ void implicitly_convertible(const std::type_info *src,
void implicitly_convertible(bool (*predicate)(PyTypeObject *, PyObject *,
cleanup_list *),
const std::type_info *dst) noexcept {
nb_type_map &type_c2p = internals->type_c2p;

nb_type_map::iterator it = type_c2p.find(std::type_index(*dst));
check(it != type_c2p.end(),
"nanobind::detail::implicitly_convertible(src=<predicate>, dst=%s): "
"destination type unknown!", type_name(dst));
type_data *t = nb_type_c2p(internals, dst);
check(t, "nanobind::detail::implicitly_convertible(src=<predicate>, dst=%s): "
"destination type unknown!", type_name(dst));

type_data *t = it->second;
size_t size = 0;

if (t->flags & (uint32_t) type_flags::has_implicit_conversions) {
Expand Down
4 changes: 2 additions & 2 deletions src/nb_func.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -942,9 +942,9 @@ static void nb_func_render_signature(const func_data *f) noexcept {
"nb::detail::nb_func_render_signature(): missing type!");

if (!(is_method && arg_index == 0)) {
auto it = internals->type_c2p.find(std::type_index(**descr_type));
auto it = internals->type_c2p_slow.find(*descr_type);

if (it != internals->type_c2p.end()) {
if (it != internals->type_c2p_slow.end()) {
handle th((PyObject *) it->second->type_py);
buf.put_dstr((borrow<str>(th.attr("__module__"))).c_str());
buf.put('.');
Expand Down
9 changes: 5 additions & 4 deletions src/nb_internals.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

/// Tracks the ABI of nanobind
#ifndef NB_INTERNALS_VERSION
# define NB_INTERNALS_VERSION 11
# define NB_INTERNALS_VERSION 12
#endif

/// On MSVC, debug and release builds are not ABI-compatible!
Expand Down Expand Up @@ -252,12 +252,13 @@ static void internals_cleanup() {
leak = true;
}

if (!internals->type_c2p.empty()) {
if (!internals->type_c2p_slow.empty() ||
!internals->type_c2p_fast.empty()) {
if (internals->print_leak_warnings) {
fprintf(stderr, "nanobind: leaked %zu types!\n",
internals->type_c2p.size());
internals->type_c2p_slow.size());
int ctr = 0;
for (const auto &kv : internals->type_c2p) {
for (const auto &kv : internals->type_c2p_slow) {
fprintf(stderr, " - leaked type \"%s\"\n", kv.second->name);
if (ctr++ == 10) {
fprintf(stderr, " - ... skipped remainder\n");
Expand Down
Loading

0 comments on commit b515b1f

Please sign in to comment.