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

fix: only check for per-thread rusage in the benchmark that requires it #1186

Merged
merged 2 commits into from
Jan 10, 2020
Merged

Conversation

mr-salty
Copy link
Contributor

@mr-salty mr-salty commented Jan 10, 2020

The single_row_throughput_benchmark doesn't use getrusage(), so
checking for it in Config unnecessarily precludes it from running
(i.e. when using bazel builds).

Instead, check in multiple_rows_cpu_benchmark which actually does care.


This change is Reviewable

@mr-salty mr-salty requested a review from coryan January 10, 2020 19:57
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 10, 2020
The `single_row_throughput_benchmark` doesn't use `getrusage()`, so
checking for it in `Config` unnecessarily precludes it from running
(i.e. when using `bazel` builds).

Instead, check in `multiple_rows_cpu_benchmark` which actually does care.
@mr-salty mr-salty changed the title fix: only check for per-thread rusage in the benchmark that requires it. fix: only check for per-thread rusage in the benchmark that requires it Jan 10, 2020
@codecov
Copy link

codecov bot commented Jan 10, 2020

Codecov Report

Merging #1186 into master will increase coverage by <.01%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1186      +/-   ##
==========================================
+ Coverage   95.63%   95.64%   +<.01%     
==========================================
  Files         174      175       +1     
  Lines       13567    13570       +3     
==========================================
+ Hits        12975    12979       +4     
+ Misses        592      591       -1
Impacted Files Coverage Δ
...cloud/spanner/benchmarks/benchmarks_config_test.cc 100% <ø> (ø) ⬆️
...ogle/cloud/spanner/benchmarks/benchmarks_config.cc 99.01% <ø> (+2.75%) ⬆️
.../spanner/benchmarks/multiple_rows_cpu_benchmark.cc 92.35% <50%> (-0.19%) ⬇️
google/cloud/spanner/instance_admin_connection.h 100% <0%> (ø)
...anner/integration_tests/client_integration_test.cc 98.47% <0%> (ø) ⬆️
google/cloud/spanner/value.h 92.8% <0%> (+0.1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 69bcc73...b6dd528. Read the comment docs.

@mr-salty mr-salty merged commit 48c95d5 into googleapis:master Jan 10, 2020
@mr-salty mr-salty deleted the rusage branch January 10, 2020 21:26
devjgm pushed a commit to devjgm/google-cloud-cpp that referenced this pull request May 7, 2020
…it (googleapis/google-cloud-cpp-spanner#1186)

* fix: only check for per-thread rusage in the benchmark that requires it

The `single_row_throughput_benchmark` doesn't use `getrusage()`, so
checking for it in `Config` unnecessarily precludes it from running
(i.e. when using `bazel` builds).

Instead, check in `multiple_rows_cpu_benchmark` which actually does care.

* oops, forgot to update the test.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants