Skip to content
This repository was archived by the owner on Dec 8, 2021. It is now read-only.

Commit fbdab30

Browse files
authored
fix: clean up / fix ClientCount logic (#1175)
* fix: clean up / fix ClientCount logic Removes the workaround for #1000 from the multiple rows benchmark. Since we're no longer adjusting min, and the `Config` class already validates `min_clients <= max_clients` we no longer need to do it here. Fixes two small bugs in the existing code (AFAICT): * `ClientCount` would usually (always?) return `min_clients` because `min_clients <= max_clients` is generally true. * No need to subtract 1 from `max_clients` when generating a random number.
1 parent 620bb7c commit fbdab30

File tree

2 files changed

+9
-21
lines changed

2 files changed

+9
-21
lines changed

google/cloud/spanner/benchmarks/multiple_rows_cpu_benchmark.cc

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -382,17 +382,10 @@ class ExperimentImpl {
382382
config.minimum_threads, config.maximum_threads)(generator_);
383383
}
384384

385-
int ClientCount(Config const& config, int thread_count) {
386-
// TODO(#1000) - avoid deadlocks with more than 100 threads per client
387-
auto const min_clients =
388-
(std::max<int>)(thread_count / 100 + 1, config.minimum_clients);
389-
auto const max_clients = config.maximum_clients;
390-
if (min_clients <= max_clients) {
391-
return min_clients;
392-
}
385+
int ClientCount(Config const& config) {
393386
std::lock_guard<std::mutex> lk(mu_);
394-
return std::uniform_int_distribution<int>(min_clients,
395-
max_clients - 1)(generator_);
387+
return std::uniform_int_distribution<int>(
388+
config.minimum_clients, config.maximum_clients)(generator_);
396389
}
397390

398391
/// Get a snapshot of the random bit generator
@@ -546,7 +539,7 @@ class ReadExperiment : public Experiment {
546539
for (int i = 0; i != config.samples; ++i) {
547540
auto const use_stubs = impl_.UseStub(config);
548541
auto const thread_count = impl_.ThreadCount(config);
549-
auto const client_count = impl_.ClientCount(config, thread_count);
542+
auto const client_count = impl_.ClientCount(config);
550543
if (use_stubs) {
551544
std::vector<std::shared_ptr<cs::internal::SpannerStub>> iteration_stubs(
552545
stubs.begin(), stubs.begin() + client_count);
@@ -763,7 +756,7 @@ class SelectExperiment : public Experiment {
763756
for (int i = 0; i != config.samples; ++i) {
764757
auto const use_stubs = impl_.UseStub(config);
765758
auto const thread_count = impl_.ThreadCount(config);
766-
auto const client_count = impl_.ClientCount(config, thread_count);
759+
auto const client_count = impl_.ClientCount(config);
767760
if (use_stubs) {
768761
std::vector<std::shared_ptr<cs::internal::SpannerStub>> iteration_stubs(
769762
stubs.begin(), stubs.begin() + client_count);
@@ -997,7 +990,7 @@ class UpdateExperiment : public Experiment {
997990
for (int i = 0; i != config.samples; ++i) {
998991
auto const use_stubs = impl_.UseStub(config);
999992
auto const thread_count = impl_.ThreadCount(config);
1000-
auto const client_count = impl_.ClientCount(config, thread_count);
993+
auto const client_count = impl_.ClientCount(config);
1001994
if (use_stubs) {
1002995
std::vector<std::shared_ptr<cs::internal::SpannerStub>> iteration_stubs(
1003996
stubs.begin(), stubs.begin() + client_count);
@@ -1261,7 +1254,7 @@ class MutationExperiment : public Experiment {
12611254
for (int i = 0; i != config.samples; ++i) {
12621255
auto const use_stubs = impl_.UseStub(config);
12631256
auto const thread_count = impl_.ThreadCount(config);
1264-
auto const client_count = impl_.ClientCount(config, thread_count);
1257+
auto const client_count = impl_.ClientCount(config);
12651258
if (use_stubs) {
12661259
std::vector<std::shared_ptr<cs::internal::SpannerStub>> iteration_stubs(
12671260
stubs.begin(), stubs.begin() + client_count);

google/cloud/spanner/benchmarks/single_row_throughput_benchmark.cc

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -214,13 +214,8 @@ void FillTable(Config const& config, cloud_spanner::Database const& database,
214214

215215
int ClientCount(Config const& config,
216216
google::cloud::internal::DefaultPRNG& generator) {
217-
auto min_clients = config.minimum_clients;
218-
auto const max_clients = config.maximum_clients;
219-
if (min_clients <= max_clients) {
220-
return min_clients;
221-
}
222-
return std::uniform_int_distribution<int>(min_clients,
223-
max_clients - 1)(generator);
217+
return std::uniform_int_distribution<int>(config.minimum_clients,
218+
config.maximum_clients)(generator);
224219
};
225220

226221
class InsertOrUpdateExperiment : public Experiment {

0 commit comments

Comments
 (0)