Build benchmark and all tests with Bazel without modifying C++ source code by qzmfranklin · Pull Request #501 · google/benchmark

qzmfranklin · 2017-12-14T21:57:33Z

This PR attempts to build all libraries and tests of benchmark with Bazel without modifying the source code.

Tested on a Ubuntu 16.04 x86_64 machine with GCC 4.7.0 and clang 5.0.0:

bazel test :all

Also, this BUILD file is written to allow this repository to be included in other Bazel based projects in source format.

Of the 17 tests, 16 are passing as of this writing.

The failed one, user_counters_tabular_test has the following error log:

Testing ConsoleReporter Output
-------------------------------
2017-12-14 21:49:08
Run on (8 X 3900 MHz CPU s)
CPU Caches:
  L1 Data 32K (x4)
  L1 Instruction 32K (x4)
  L2 Unified 256K (x4)
  L3 Unified 8192K (x1)
***WARNING*** CPU scaling is enabled, the benchmark real time measurements may be noisy and will incur extra overhead.
***WARNING*** Library was built as DEBUG. Timings may be affected.
test/output_test_helper.cc:97: CheckCase: Check `TC.match_rule != MR_Next' failed. Expected line "Benchmark                                   Time           CPU Iterations UserCounters..." to match regex "^Benchmark %s Time %s CPU %s Iterations %s Bar %s Bat %s Baz %s Foo %s Frob %s Lob$"
    actual regex string "^Benchmark[ ]+Time[ ]+CPU[ ]+Iterations[ ]+Bar[ ]+Bat[ ]+Baz[ ]+Foo[ ]+Frob[ ]+Lob$"
    started matching near: Benchmark                                   Time           CPU Iterations UserCounters...
------------------------------------------------------------------------------------------
Benchmark                                   Time           CPU Iterations UserCounters...
------------------------------------------------------------------------------------------
BM_Counters_Tabular/threads:1               2 ns          2 ns  287283167 Bar=2 Bat=8 Baz=4 Foo=1 Frob=16 Lob=32
BM_Counters_Tabular/threads:2               1 ns          3 ns  296119936 Bar=2 Bat=8 Baz=4 Foo=1 Frob=16 Lob=32
BM_Counters_Tabular/threads:4               1 ns          2 ns  273998224 Bar=2 Bat=8 Baz=4 Foo=1 Frob=16 Lob=32
BM_Counters_Tabular/threads:8               0 ns          3 ns  285606144 Bar=2 Bat=8 Baz=4 Foo=1 Frob=16 Lob=32
BM_Counters_Tabular/threads:16              0 ns          3 ns  278433088 Bar=2 Bat=8 Baz=4 Foo=1 Frob=16 Lob=32
BM_CounterRates_Tabular/threads:1           2 ns          2 ns  350217774 Bar=2.8575/s Bat=11.43/s Baz=5.71499/s Foo=1.42875/s Frob=22.86/s Lob=45.72/s
BM_CounterRates_Tabular/threads:2           1 ns          3 ns  200000000 Bar=3.71317/s Bat=14.8527/s Baz=7.42633/s Foo=1.85658/s Frob=29.7053/s Lob=59.4107/s
BM_CounterRates_Tabular/threads:4           1 ns          3 ns  272143792 Bar=2.82876/s Bat=11.315/s Baz=5.65752/s Foo=1.41438/s Frob=22.6301/s Lob=45.2601/s
BM_CounterRates_Tabular/threads:8           0 ns          3 ns  281718824 Bar=2.82473/s Bat=11.2989/s Baz=5.64946/s Foo=1.41237/s Frob=22.5979/s Lob=45.1957/s
BM_CounterRates_Tabular/threads:16          0 ns          3 ns  260389536 Bar=3.04074/s Bat=12.163/s Baz=6.08148/s Foo=1.52037/s Frob=24.3259/s Lob=48.6519/s
BM_CounterSet0_Tabular/threads:1            2 ns          2 ns  317143900 Bar=20 Baz=40 Foo=10
BM_CounterSet0_Tabular/threads:2            1 ns          3 ns  200000000 Bar=20 Baz=40 Foo=10
BM_CounterSet0_Tabular/threads:4            1 ns          2 ns  266893560 Bar=20 Baz=40 Foo=10
BM_CounterSet0_Tabular/threads:8            0 ns          2 ns  272857264 Bar=20 Baz=40 Foo=10
BM_CounterSet0_Tabular/threads:16           0 ns          3 ns  272636032 Bar=20 Baz=40 Foo=10
BM_CounterSet1_Tabular/threads:1            2 ns          2 ns  338089421 Bar=25 Baz=45 Foo=15
BM_CounterSet1_Tabular/threads:2            1 ns          2 ns  340538902 Bar=25 Baz=45 Foo=15
BM_CounterSet1_Tabular/threads:4            1 ns          3 ns  271316996 Bar=25 Baz=45 Foo=15
BM_CounterSet1_Tabular/threads:8            1 ns          3 ns  281292888 Bar=25 Baz=45 Foo=15
BM_CounterSet1_Tabular/threads:16           0 ns          3 ns  271893952 Bar=25 Baz=45 Foo=15
BM_CounterSet2_Tabular/threads:1            2 ns          2 ns  302572860 Bat=30 Baz=40 Foo=10
BM_CounterSet2_Tabular/threads:2            1 ns          3 ns  200000000 Bat=30 Baz=40 Foo=10
BM_CounterSet2_Tabular/threads:4            1 ns          2 ns  289976960 Bat=30 Baz=40 Foo=10
BM_CounterSet2_Tabular/threads:8            0 ns          2 ns  284623584 Bat=30 Baz=40 Foo=10
BM_CounterSet2_Tabular/threads:16           0 ns          3 ns  277888528 Bat=30 Baz=40 Foo=10
Aborted (core dumped)

dmah42 · 2017-12-14T22:28:17Z

see #496

how is this different?

coveralls · 2017-12-14T22:54:01Z

Coverage remained the same at 86.926% when pulling 39ffb95 on qzmfranklin:bazel into e4ccad7 on google:master.

AppVeyorBot · 2017-12-14T23:14:27Z

✅ Build benchmark 909 completed (commit 900c2f7d30 by @qzmfranklin)

qzmfranklin · 2017-12-15T04:40:33Z

Hi @dominichamon ,

Thanks for the reply!

I saw the one you listed before I created this PR.
I think these two PRs have the same spirit of bazeling the build. Here are some differences:

This PR does not modify the existing source code at all, thus minimizing the impact on the existing build system. The other one needs to modify the source code and has thus broken a few unit tests.
The BUILD file of this PR is only 69 lines of code. The other one was 392 lines of code. They achieved the same effects.

Given the above, I believed there was still value in creating this PR.

How do you think?

Thanks,
Zhongming

dmah42 · 2017-12-18T16:41:13Z

BUILD

+    ],
+    copts = benchmark_copts,
+    deps = [
+        ':output_test_helper',


these should also depend on :benchmark, right?

Yes. But we do not need to specify that dependency here because output_test_helper already depends on :benchmark.

dmah42 · 2017-12-18T16:41:36Z

BUILD

+    copts = benchmark_copts,
+    deps = [
+        ':benchmark',
+        gtest_dep,


i don't think any of these need gtest

Thanks! Will remove with the next update.

dmah42 · 2017-12-18T16:41:47Z

BUILD

+    ],
+    deps = [
+        ':benchmark',
+        gtest_dep,


doesn't use gtest, afaik.

Thanks! Removed. Will update with the next update.

dmah42 · 2017-12-18T16:43:38Z

WORKSPACE

@@ -0,0 +1,69 @@
+new_git_repository(


i don't know enough bazel: is this defining the googletest repo dependency for use in the BUILD? or is it supposed to define the benchmark package and this is a bad copy-paste?

The former.

Here is an explanation of how the new_git_repository rule registers a git repository as a dependency:
https://docs.bazel.build/versions/master/be/workspace.html#git_repository

But I believe you might have had question regarding the comment of copying the content of the BUILD file. The in-source comment is, to the best of my knowledge, stating the facts and rationale. Hopefully with a look at the git_repository rule that I pasted above, it clears out for you and potentially other reviewers.

dmah42 · 2017-12-18T16:44:11Z

can you add a travis bazel build?

qzmfranklin · 2017-12-19T03:51:53Z

Will update with a Bazel build test for Travis. Though, at this very moment, user_counters_tabular_test fails. I will add it and hopefully get some guidance from you as to what can be done to fix that test.

coveralls · 2017-12-19T03:59:42Z

Coverage remained the same at 86.926% when pulling a1f5d43 on qzmfranklin:bazel into e4ccad7 on google:master.

qzmfranklin · 2017-12-19T04:02:29Z

OK, before that, the Travis bot got a case-sensitivity issue. This run failed to create the build directory:

$ mkdir -p build && cd build

mkdir: build: Not a directory

This is because the filesystem it uses is case-insensitive and this PR is adding a file named BUILD. Is there a way to make the filesystem used in the Travis bot case-sensitive?

I have found a few things on Travis:
travis-ci/docs-travis-ci-com#209

Looks like they just do not have case-sensitivity be default on some bots.

I am going to rename BUILD to BUILD.bazel for now. There is no downside, AFAIK, to using BUILD.bazel other than the fact that the file name is longer.

The rationale for making this change is docmented here: google#501 Briefly speaking, some test bots on TravisCI do not have case sensitive filesystems. So the build will fail to create the `build directory`. It does not look like Travis is going to resolve this problem on their side anytime soon, yet. So I am making this change to pass the test.

coveralls · 2017-12-19T04:18:05Z

Coverage remained the same at 86.926% when pulling 1fce7a8 on qzmfranklin:bazel into e4ccad7 on google:master.

AppVeyorBot · 2017-12-19T04:54:50Z

✅ Build benchmark 910 completed (commit 538a966059 by @qzmfranklin)

By using a different fork, for now, with the understanding that we should switch to the google official googletest repo as soon as the issue below is resolved: google/googletest#1212

AppVeyorBot · 2017-12-19T05:56:31Z

✅ Build benchmark 916 completed (commit c706dc2345 by @qzmfranklin)

qzmfranklin · 2017-12-19T06:04:58Z

@dominichamon

Urrhh, not successful with adding the bazel test yet. Somehow it fails on TravisCI bot.

I need to go to sleep soon. Will continue tomorrow.

Thanks for all your advice so far. Really appreciate them.

AppVeyorBot · 2017-12-19T07:00:43Z

✅ Build benchmark 921 completed (commit e602f99d9b by @qzmfranklin)

dmah42 · 2017-12-19T17:46:59Z

you can also change the build directory to be _build if that helps.

qzmfranklin · 2017-12-20T07:07:23Z

Have not got time to work on this one tonight. Will try tomorrow.

dmah42 · 2018-01-11T18:26:33Z

Just checking in.. did you have a chance to look again?

dmah42 · 2018-03-08T12:49:22Z

Made obsolete by #533

qzmfranklin added 2 commits December 14, 2017 13:53

Add Bazel BUILD.

9dbce74

Fully Bazelise the build and tests.

39ffb95

googlebot added the cla: yes label Dec 14, 2017

qzmfranklin changed the title ~~Build benchmark and all tests with Bazel~~ Build benchmark and all tests with Bazel without modifying C++ source code Dec 14, 2017

dmah42 reviewed Dec 18, 2017

View reviewed changes

Remove unused gtest dependencies.

a1f5d43

qzmfranklin force-pushed the bazel branch 4 times, most recently from 1feb9ab to 731423f Compare December 19, 2017 04:44

qzmfranklin force-pushed the bazel branch from 731423f to 1aa9146 Compare December 19, 2017 04:57

Add Bazel tests to Travis

ba133ba

qzmfranklin force-pushed the bazel branch from 1aa9146 to ba133ba Compare December 19, 2017 05:14

Fix the WORKSPACE file.

4fa07d2

qzmfranklin force-pushed the bazel branch from 8882888 to 4fa07d2 Compare December 19, 2017 05:28

Work around an issue in the googletest repo.

acf6319

By using a different fork, for now, with the understanding that we should switch to the google official googletest repo as soon as the issue below is resolved: google/googletest#1212

jmarantz mentioned this pull request Jan 11, 2018

utility: add benchmark library, but don't use it yet. envoyproxy/envoy#2350

Merged

jmillikin-stripe mentioned this pull request Feb 21, 2018

Add support for building with Bazel. #533

Merged

dmah42 closed this Mar 8, 2018

Conversation

qzmfranklin commented Dec 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dmah42 commented Dec 14, 2017

Uh oh!

coveralls commented Dec 14, 2017

Uh oh!

AppVeyorBot commented Dec 14, 2017

Uh oh!

qzmfranklin commented Dec 15, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmah42 commented Dec 18, 2017

Uh oh!

qzmfranklin commented Dec 19, 2017

Uh oh!

coveralls commented Dec 19, 2017

Uh oh!

qzmfranklin commented Dec 19, 2017

Uh oh!

coveralls commented Dec 19, 2017

Uh oh!

AppVeyorBot commented Dec 19, 2017

Uh oh!

AppVeyorBot commented Dec 19, 2017

Uh oh!

qzmfranklin commented Dec 19, 2017

Uh oh!

AppVeyorBot commented Dec 19, 2017

Uh oh!

dmah42 commented Dec 19, 2017

Uh oh!

qzmfranklin commented Dec 20, 2017

Uh oh!

dmah42 commented Jan 11, 2018

Uh oh!

dmah42 commented Mar 8, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

qzmfranklin commented Dec 14, 2017 •

edited

Loading