From cd95cf15d483738b24fad889b8844e787204d1fa Mon Sep 17 00:00:00 2001 From: Philipp Moritz Date: Sun, 13 May 2018 19:39:58 -0700 Subject: [PATCH 1/8] use mersenne twister to generate random number --- cpp/src/plasma/common.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cpp/src/plasma/common.cc b/cpp/src/plasma/common.cc index 2de06d5f8cf..7a77f0bd1f1 100644 --- a/cpp/src/plasma/common.cc +++ b/cpp/src/plasma/common.cc @@ -28,9 +28,10 @@ using arrow::Status; UniqueID UniqueID::from_random() { UniqueID id; uint8_t* data = id.mutable_data(); - std::random_device engine; + static thread_local std::mt19937 generator; + std::uniform_int_distribution d(0, std::numeric_limits::max()); for (int i = 0; i < kUniqueIDSize; i++) { - data[i] = static_cast(engine()); + data[i] = static_cast(d(generator)); } return id; } From 841a67f01bb4ceec3a3b701a6874e1886495ad2f Mon Sep 17 00:00:00 2001 From: Philipp Moritz Date: Sun, 13 May 2018 20:04:12 -0700 Subject: [PATCH 2/8] fix linting --- cpp/src/plasma/common.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/cpp/src/plasma/common.cc b/cpp/src/plasma/common.cc index 7a77f0bd1f1..f97500a5c21 100644 --- a/cpp/src/plasma/common.cc +++ b/cpp/src/plasma/common.cc @@ -17,6 +17,7 @@ #include "plasma/common.h" +#include #include #include "plasma/plasma_generated.h" From 62d412f3c08729e7e4b8f86a2fdd5997e7d7f16d Mon Sep 17 00:00:00 2001 From: Philipp Moritz Date: Sun, 13 May 2018 21:08:31 -0700 Subject: [PATCH 3/8] fix on older versions of macOS --- cpp/src/plasma/common.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cpp/src/plasma/common.cc b/cpp/src/plasma/common.cc index f97500a5c21..a7528d91a5a 100644 --- a/cpp/src/plasma/common.cc +++ b/cpp/src/plasma/common.cc @@ -29,7 +29,12 @@ using arrow::Status; UniqueID UniqueID::from_random() { UniqueID id; uint8_t* data = id.mutable_data(); - static thread_local std::mt19937 generator; + // 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 mutex; + std::lock_guard lock(mutex); + static std::mt19937 generator; std::uniform_int_distribution d(0, std::numeric_limits::max()); for (int i = 0; i < kUniqueIDSize; i++) { data[i] = static_cast(d(generator)); From f60bd99c7d4f10ac78ca70e3676fba6ecf0bbfe4 Mon Sep 17 00:00:00 2001 From: Philipp Moritz Date: Sun, 13 May 2018 21:14:59 -0700 Subject: [PATCH 4/8] more valgrind fixes --- cpp/src/plasma/test/client_tests.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/src/plasma/test/client_tests.cc b/cpp/src/plasma/test/client_tests.cc index b80862d95b4..edabf66c54e 100644 --- a/cpp/src/plasma/test/client_tests.cc +++ b/cpp/src/plasma/test/client_tests.cc @@ -51,8 +51,8 @@ class TestPlasmaStore : public ::testing::Test { // TODO(pcm): At the moment, stdout of the test gets mixed up with // stdout of the object store. Consider changing that. void SetUp() { - std::mt19937 rng; - rng.seed(std::random_device()()); + auto seed = std::chrono::high_resolution_clock::now().time_since_epoch().count(); + std::mt19937 rng(seed); std::string store_index = std::to_string(rng()); store_socket_name_ = "/tmp/store" + store_index; From 83740b5c6d231129271548a7b89aea579d093766 Mon Sep 17 00:00:00 2001 From: Philipp Moritz Date: Sun, 13 May 2018 21:24:22 -0700 Subject: [PATCH 5/8] update --- cpp/src/plasma/common.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cpp/src/plasma/common.cc b/cpp/src/plasma/common.cc index a7528d91a5a..0bd7386e621 100644 --- a/cpp/src/plasma/common.cc +++ b/cpp/src/plasma/common.cc @@ -18,7 +18,9 @@ #include "plasma/common.h" #include +#include #include +#include #include "plasma/plasma_generated.h" From beb5bab8f396afa933afcd0317c63e83cd034bf7 Mon Sep 17 00:00:00 2001 From: Philipp Moritz Date: Sun, 13 May 2018 22:47:20 -0700 Subject: [PATCH 6/8] update --- cpp/src/plasma/test/client_tests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/plasma/test/client_tests.cc b/cpp/src/plasma/test/client_tests.cc index edabf66c54e..9b51baac1ee 100644 --- a/cpp/src/plasma/test/client_tests.cc +++ b/cpp/src/plasma/test/client_tests.cc @@ -51,7 +51,7 @@ class TestPlasmaStore : public ::testing::Test { // TODO(pcm): At the moment, stdout of the test gets mixed up with // stdout of the object store. Consider changing that. void SetUp() { - auto seed = std::chrono::high_resolution_clock::now().time_since_epoch().count(); + uint64_t seed = std::chrono::high_resolution_clock::now().time_since_epoch().count(); std::mt19937 rng(seed); std::string store_index = std::to_string(rng()); store_socket_name_ = "/tmp/store" + store_index; From be4bb84d89efb3f85fd264a53a652733cde0a515 Mon Sep 17 00:00:00 2001 From: Philipp Moritz Date: Sun, 13 May 2018 22:58:24 -0700 Subject: [PATCH 7/8] fix --- cpp/src/plasma/test/client_tests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/plasma/test/client_tests.cc b/cpp/src/plasma/test/client_tests.cc index 9b51baac1ee..23d2c2b1a53 100644 --- a/cpp/src/plasma/test/client_tests.cc +++ b/cpp/src/plasma/test/client_tests.cc @@ -52,7 +52,7 @@ class TestPlasmaStore : public ::testing::Test { // stdout of the object store. Consider changing that. void SetUp() { uint64_t seed = std::chrono::high_resolution_clock::now().time_since_epoch().count(); - std::mt19937 rng(seed); + std::mt19937 rng(static_cast(seed)); std::string store_index = std::to_string(rng()); store_socket_name_ = "/tmp/store" + store_index; From 21d0e3f77e1e30b2e799c0bc8f0e36327e60ab02 Mon Sep 17 00:00:00 2001 From: Philipp Moritz Date: Sun, 13 May 2018 23:44:42 -0700 Subject: [PATCH 8/8] fixes --- cpp/src/plasma/common.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cpp/src/plasma/common.cc b/cpp/src/plasma/common.cc index 0bd7386e621..be3fc74b062 100644 --- a/cpp/src/plasma/common.cc +++ b/cpp/src/plasma/common.cc @@ -20,7 +20,6 @@ #include #include #include -#include #include "plasma/plasma_generated.h" @@ -37,9 +36,9 @@ UniqueID UniqueID::from_random() { static std::mutex mutex; std::lock_guard lock(mutex); static std::mt19937 generator; - std::uniform_int_distribution d(0, std::numeric_limits::max()); + std::uniform_int_distribution dist(0, std::numeric_limits::max()); for (int i = 0; i < kUniqueIDSize; i++) { - data[i] = static_cast(d(generator)); + data[i] = static_cast(dist(generator)); } return id; }