Skip to content

Commit

Permalink
perf(TS): Remove redundant instructions from tile hash (#3898)
Browse files Browse the repository at this point in the history
Optimises the code in the tile hash function. During a render this
function is called billions of times so this small improvement adds up,
in some test scenes it reduces total render time by an average of about
2%.

See https://godbolt.org/z/jMzz4PTce for the exact details. A summary
would be:

- Reduced instruction count from 75 to 61, this is due to the redundant
  add and shift operations that can be implicitely handled by taking
  advantage of the member layout in the class.
- Halves the number of memory fetch instructions because all accesses
  are now 64-bit.
- Removes branching instructions and extra memory indirection inside
  `ustring::hash()`.

There is a slight change in behaviour, which is that the hash is now
computed from the `m_file` pointer rather than from
`m_file->filename().hash()`, so runtime reproducibility is lost. However
the performance gain seems worth it.

Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Larry Gritz <[email protected]>
Co-authored-by: Larry Gritz <[email protected]>
  • Loading branch information
curtisblack and lgritz committed Jul 25, 2023
1 parent 9fd9fbf commit d757bff
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 7 deletions.
5 changes: 5 additions & 0 deletions src/include/OpenImageIO/hash.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,12 @@ inline uint64_t fasthash64(const void *buf, size_t len, uint64_t seed=1771)
uint64_t v;

while (pos != end) {
// This appears to be a false positive which only affects GCC.
// https://godbolt.org/z/5q7Y7ndfb
OIIO_PRAGMA_WARNING_PUSH
OIIO_GCC_ONLY_PRAGMA(GCC diagnostic ignored "-Wmaybe-uninitialized")
v = *pos++;
OIIO_PRAGMA_WARNING_PUSH
h ^= mix(v);
h *= m;
}
Expand Down
19 changes: 12 additions & 7 deletions src/libtexture/imagecache_pvt.h
Original file line number Diff line number Diff line change
Expand Up @@ -560,13 +560,17 @@ struct TileID {
/// Digest the TileID into a size_t to use as a hash key.
size_t hash() const
{
const uint64_t a = (uint64_t(m_x) << 32) + uint64_t(m_y);
const uint64_t b = (uint64_t(m_z) << 32) + uint64_t(m_subimage);
const uint64_t c = (uint64_t(m_miplevel) << 32)
+ (uint64_t(m_chbegin) << 16) + uint64_t(m_chend);
const uint64_t d = m_file->filename().hash()
+ uint64_t(m_colortransformid);
return fasthash::fasthash64({ a, b, c, d });
static constexpr size_t member_size
= sizeof(m_x) + sizeof(m_y) + sizeof(m_z) + sizeof(m_subimage)
+ sizeof(m_miplevel) + sizeof(m_chbegin) + sizeof(m_chend)
+ sizeof(m_colortransformid) + sizeof(m_padding) + sizeof(m_file);
static_assert(
sizeof(*this) == member_size,
"All TileID members must be accounted for so we can hash the entire class.");
static_assert(
sizeof(*this) % sizeof(uint64_t) == 0,
"FastHash uses the fewest instructions when data size is a multiple of 8 bytes.");
return fasthash::fasthash64(this, sizeof(*this));
}

/// Functor that hashes a TileID
Expand All @@ -590,6 +594,7 @@ struct TileID {
int m_miplevel; ///< MIP-map level
short m_chbegin, m_chend; ///< Channel range
int m_colortransformid; ///< Colorspace id (0 == default)
int m_padding = 0; ///< Unused
ImageCacheFile* m_file; ///< Which ImageCacheFile we refer to
};

Expand Down

0 comments on commit d757bff

Please sign in to comment.