Make PGO builds deterministic again

some environment dependent execution (e.g. pid) were being std::hash'ed, and
net filenames put in unordered maps.  Also uses sprintf instead of
std::to_string.  Depending on precise content, this could lead to different
PGO'ed binaries. This is mitigated by using a basic hash function.

This also fixes a potential issue in net filename generation, in cases where
std::hash would use invocation dependent salt, which is not the cases today for
typical std libraries.

Closes https://github.com/official-stockfish/Stockfish/pull/6562

No functional change
This commit is contained in:
anematode
2026-01-22 19:15:17 +01:00
committed by Joost VandeVondele
parent f61d4317a3
commit c0f245303b
4 changed files with 50 additions and 36 deletions
+28 -13
View File
@@ -34,6 +34,7 @@
#include <memory>
#include <string>
#include <string_view>
#include <type_traits>
#include <vector>
#define stringify2(x) #x
@@ -305,24 +306,38 @@ inline uint64_t mul_hi64(uint64_t a, uint64_t b) {
#endif
}
template<typename T>
inline void hash_combine(std::size_t& seed, const T& v) {
std::hash<T> hasher;
seed ^= hasher(v) + 0x9e3779b9 + (seed << 6) + (seed >> 2);
}
template<>
inline void hash_combine(std::size_t& seed, const std::size_t& v) {
seed ^= v + 0x9e3779b9 + (seed << 6) + (seed >> 2);
inline std::uint64_t hash_bytes(const char* data, std::size_t size) {
// FNV-1a 64-bit
const char* p = data;
std::uint64_t h = 14695981039346656037ull;
for (std::size_t i = 0; i < size; ++i)
h = (h ^ p[i]) * 1099511628211ull;
return h;
}
template<typename T>
inline std::size_t get_raw_data_hash(const T& value) {
return std::hash<std::string_view>{}(
std::string_view(reinterpret_cast<const char*>(&value), sizeof(value)));
// We must have no padding bytes because we're reinterpreting as char
static_assert(std::has_unique_object_representations<T>());
return static_cast<std::size_t>(
hash_bytes(reinterpret_cast<const char*>(&value), sizeof(value)));
}
template<typename T>
inline void hash_combine(std::size_t& seed, const T& v) {
std::size_t x;
// For primitive types we avoid using the default hasher, which may be
// nondeterministic across program invocations
if constexpr (std::is_integral<T>())
x = v;
else
x = std::hash<T>{}(v);
seed ^= x + 0x9e3779b9 + (seed << 6) + (seed >> 2);
}
inline std::uint64_t hash_string(const std::string& sv) { return hash_bytes(sv.data(), sv.size()); }
template<std::size_t Capacity>
class FixedString {
public:
@@ -450,7 +465,7 @@ void move_to_front(std::vector<T>& vec, Predicate pred) {
template<std::size_t N>
struct std::hash<Stockfish::FixedString<N>> {
std::size_t operator()(const Stockfish::FixedString<N>& fstr) const noexcept {
return std::hash<std::string_view>{}((std::string_view) fstr);
return Stockfish::hash_bytes(fstr.data(), fstr.size());
}
};
+1 -1
View File
@@ -1586,7 +1586,7 @@ class LazyNumaReplicatedSystemWide: public NumaReplicatedBase {
CpuIndex cpu = *cfg.nodes[idx].begin(); // get a CpuIndex from NumaIndex
NumaIndex sys_idx = cfg_sys.is_cpu_assigned(cpu) ? cfg_sys.nodeByCpu.at(cpu) : 0;
std::string s = cfg_sys.to_string() + "$" + std::to_string(sys_idx);
return std::hash<std::string>{}(s);
return static_cast<std::size_t>(hash_string(s));
}
void ensure_present(NumaIndex idx) const {
+8 -10
View File
@@ -20,6 +20,7 @@
#define SHM_H_INCLUDED
#include <algorithm>
#include <cinttypes>
#include <cstddef>
#include <cstdint>
#include <cstring>
@@ -512,12 +513,9 @@ template<typename T>
struct SystemWideSharedConstant {
private:
static std::string createHashString(const std::string& input) {
size_t hash = std::hash<std::string>{}(input);
std::stringstream ss;
ss << std::hex << std::setfill('0') << hash;
return ss.str();
char buf[1024];
std::snprintf(buf, sizeof(buf), "%016" PRIx64, hash_string(input));
return buf;
}
public:
@@ -534,11 +532,11 @@ struct SystemWideSharedConstant {
// that are not present in the content, for example NUMA node allocation.
SystemWideSharedConstant(const T& value, std::size_t discriminator = 0) {
std::size_t content_hash = std::hash<T>{}(value);
std::size_t executable_hash = std::hash<std::string>{}(getExecutablePathHash());
std::size_t executable_hash = hash_string(getExecutablePathHash());
std::string shm_name = std::string("Local\\sf_") + std::to_string(content_hash) + "$"
+ std::to_string(executable_hash) + "$"
+ std::to_string(discriminator);
char buf[1024];
std::snprintf(buf, sizeof(buf), "Local\\sf_%zu$%zu$%zu", content_hash, executable_hash, discriminator);
std::string shm_name = buf;
#if !defined(_WIN32)
// POSIX shared memory names must start with a slash
+13 -12
View File
@@ -33,7 +33,6 @@
#include <string>
#include <inttypes.h>
#include <type_traits>
#include <unordered_set>
#include <fcntl.h>
#include <signal.h>
@@ -51,6 +50,7 @@
#define SF_MAX_SEM_NAME_LEN 255
#endif
#include "misc.h"
namespace Stockfish::shm {
@@ -74,17 +74,18 @@ class SharedMemoryBase {
class SharedMemoryRegistry {
private:
static std::mutex registry_mutex_;
static std::unordered_set<SharedMemoryBase*> active_instances_;
static std::vector<SharedMemoryBase*> active_instances_;
public:
static void register_instance(SharedMemoryBase* instance) {
std::scoped_lock lock(registry_mutex_);
active_instances_.insert(instance);
active_instances_.push_back(instance);
}
static void unregister_instance(SharedMemoryBase* instance) {
std::scoped_lock lock(registry_mutex_);
active_instances_.erase(instance);
active_instances_.erase(
std::remove(active_instances_.begin(), active_instances_.end(), instance), active_instances_.end());
}
static void cleanup_all(bool skip_unmap = false) noexcept {
@@ -96,7 +97,7 @@ class SharedMemoryRegistry {
};
inline std::mutex SharedMemoryRegistry::registry_mutex_;
inline std::unordered_set<SharedMemoryBase*> SharedMemoryRegistry::active_instances_;
inline std::vector<SharedMemoryBase*> SharedMemoryRegistry::active_instances_;
class CleanupHooks {
private:
@@ -181,9 +182,10 @@ class SharedMemory: public detail::SharedMemoryBase {
}
static std::string make_sentinel_base(const std::string& name) {
uint64_t hash = std::hash<std::string>{}(name);
char buf[32];
std::snprintf(buf, sizeof(buf), "sfshm_%016" PRIx64, static_cast<uint64_t>(hash));
// Using std::to_string here causes non-deterministic PGO builds.
// snprintf, being part of libc, is insensitive to the formatted values.
std::snprintf(buf, sizeof(buf), "sfshm_%016" PRIu64, hash_string(name));
return buf;
}
@@ -442,11 +444,10 @@ class SharedMemory: public detail::SharedMemoryBase {
}
std::string sentinel_full_path(pid_t pid) const {
std::string path = "/dev/shm/";
path += sentinel_base_;
path.push_back('.');
path += std::to_string(pid);
return path;
char buf[1024];
// See above snprintf comment
std::snprintf(buf, sizeof(buf), "/dev/shm/%s.%ld", sentinel_base_.c_str(), long(pid));
return buf;
}
void decrement_refcount_relaxed() noexcept {