[xray] Fix valgrind crash when memory profiling raylet (#2583)

* use different random number generator to be compatible with older valgrind versions

* seed from time

* style

* fix

* remove more random devices

* also remove random_device from global scheduler

* rename mutex

* linting
This commit is contained in:
Philipp Moritz 2018-08-09 15:37:17 -07:00 committed by Stephanie Wang
parent f093ed1fc6
commit 143a118fbf
4 changed files with 31 additions and 9 deletions

View file

@ -1,6 +1,7 @@
#ifndef GLOBAL_SCHEDULER_ALGORITHM_H #ifndef GLOBAL_SCHEDULER_ALGORITHM_H
#define GLOBAL_SCHEDULER_ALGORITHM_H #define GLOBAL_SCHEDULER_ALGORITHM_H
#include <chrono>
#include <random> #include <random>
#include "common.h" #include "common.h"
@ -25,9 +26,16 @@ enum class GlobalSchedulerAlgorithm {
class GlobalSchedulerPolicyState { class GlobalSchedulerPolicyState {
public: public:
GlobalSchedulerPolicyState(int64_t round_robin_index) GlobalSchedulerPolicyState(int64_t round_robin_index)
: round_robin_index_(round_robin_index), gen_(rd_()) {} : round_robin_index_(round_robin_index),
gen_(std::chrono::high_resolution_clock::now()
.time_since_epoch()
.count()) {}
GlobalSchedulerPolicyState() : round_robin_index_(0), gen_(rd_()) {} GlobalSchedulerPolicyState()
: round_robin_index_(0),
gen_(std::chrono::high_resolution_clock::now()
.time_since_epoch()
.count()) {}
/// Return the policy's random number generator. /// Return the policy's random number generator.
/// ///
@ -42,8 +50,6 @@ class GlobalSchedulerPolicyState {
private: private:
/// The index of the next local scheduler to assign a task to. /// The index of the next local scheduler to assign a task to.
int64_t round_robin_index_; int64_t round_robin_index_;
/// Internally maintained random number engine device.
std::random_device rd_;
/// Internally maintained random number generator. /// Internally maintained random number generator.
std::mt19937_64 gen_; std::mt19937_64 gen_;
}; };

View file

@ -1,6 +1,9 @@
#include "ray/id.h" #include "ray/id.h"
#include <limits.h> #include <limits.h>
#include <chrono>
#include <mutex>
#include <random> #include <random>
#include "ray/constants.h" #include "ray/constants.h"
@ -8,6 +11,12 @@
namespace ray { namespace ray {
std::mt19937 RandomlySeededMersenneTwister() {
auto seed = std::chrono::high_resolution_clock::now().time_since_epoch().count();
std::mt19937 seeded_engine(seed);
return seeded_engine;
}
UniqueID::UniqueID(const plasma::UniqueID &from) { UniqueID::UniqueID(const plasma::UniqueID &from) {
std::memcpy(&id_, from.data(), kUniqueIDSize); std::memcpy(&id_, from.data(), kUniqueIDSize);
} }
@ -15,9 +24,15 @@ UniqueID::UniqueID(const plasma::UniqueID &from) {
UniqueID UniqueID::from_random() { UniqueID UniqueID::from_random() {
UniqueID id; UniqueID id;
uint8_t *data = id.mutable_data(); uint8_t *data = id.mutable_data();
std::random_device engine; // NOTE(pcm): The right way to do this is to have one std::mt19937 per
// thread (using the thread_local keyword), but that's not supported on
// older versions of macOS (see https://stackoverflow.com/a/29929949)
static std::mutex random_engine_mutex;
std::lock_guard<std::mutex> lock(random_engine_mutex);
static std::mt19937 generator = RandomlySeededMersenneTwister();
std::uniform_int_distribution<uint32_t> dist(0, std::numeric_limits<uint8_t>::max());
for (int i = 0; i < kUniqueIDSize; i++) { for (int i = 0; i < kUniqueIDSize; i++) {
data[i] = static_cast<uint8_t>(engine()); data[i] = static_cast<uint8_t>(dist(generator));
} }
return id; return id;
} }

View file

@ -1,5 +1,7 @@
#include "scheduling_policy.h" #include "scheduling_policy.h"
#include <chrono>
#include "ray/util/logging.h" #include "ray/util/logging.h"
namespace ray { namespace ray {
@ -7,7 +9,8 @@ namespace ray {
namespace raylet { namespace raylet {
SchedulingPolicy::SchedulingPolicy(const SchedulingQueue &scheduling_queue) SchedulingPolicy::SchedulingPolicy(const SchedulingQueue &scheduling_queue)
: scheduling_queue_(scheduling_queue), gen_(rd_()) {} : scheduling_queue_(scheduling_queue),
gen_(std::chrono::high_resolution_clock::now().time_since_epoch().count()) {}
std::unordered_map<TaskID, ClientID> SchedulingPolicy::Schedule( std::unordered_map<TaskID, ClientID> SchedulingPolicy::Schedule(
const std::unordered_map<ClientID, SchedulingResources> &cluster_resources, const std::unordered_map<ClientID, SchedulingResources> &cluster_resources,

View file

@ -38,8 +38,6 @@ class SchedulingPolicy {
private: private:
/// An immutable reference to the scheduling task queues. /// An immutable reference to the scheduling task queues.
const SchedulingQueue &scheduling_queue_; const SchedulingQueue &scheduling_queue_;
/// Internally maintained random number engine device.
std::random_device rd_;
/// Internally maintained random number generator. /// Internally maintained random number generator.
std::mt19937_64 gen_; std::mt19937_64 gen_;
}; };