Commit Graph

340 Commits

Author SHA1 Message Date
Vy Nguyen
a3235d7b69
Include the benchmark's family-name in State (#1511)
* Include the benchmark's family-name in State

For compat with internal library, where State::name() returns the benchmark's family name.

* added missing files from prev commit

* fix field-init order error

* added test
2023-01-10 16:48:17 +00:00
SunBlack
fe65457e80
Fix typos found by codespell (#1519) 2023-01-10 12:25:32 +00:00
SunBlack
37faf6f975
Fix Clang-Tidy warnings related to modernize-use-override (#1523) 2023-01-09 17:52:18 +00:00
Yury Fedorov
62edc4fb00
Bug fix variable 'actual_iterations' set but not used (#1517)
* Bug fix variable 'actual_iterations' set but not used

Compiling the project in clang 15 without -Wno-unused-but-set-variable flag the following error is generated:

benchmark-src/test/options_test.cc:70:10: error: variable 'actual_iterations' set but not used [-Werror,-Wunused-but-set-variable]
  size_t actual_iterations = 0;
         ^

* Adjust according formatting of `clang-format`

Co-authored-by: dominic hamon <510002+dmah42@users.noreply.github.com>
2022-12-19 14:03:11 +00:00
Vy Nguyen
9714eb8d11
Removed deprecated function (#1506)
* Removed deprecated function

* updated tests too

* restore comment

Co-authored-by: dominic hamon <dominichamon@users.noreply.github.com>
2022-11-11 15:12:12 +00:00
Roman Lebedev
db4f581fbb
Partially revert "Do not depend on unversioned python binary (#1496)" (#1501)
As predicted, the cmake part of the change is contentious.
https://github.com/google/benchmark/pull/1496#issuecomment-1276508266

This partially reverts commit 229bc5a937.
2022-10-13 10:03:29 +01:00
Matthias Braun
229bc5a937
Do not depend on unversioned python binary (#1496)
Some linux distributions no longer provide `python` binary and require
usage of `python3` instead. This changes the scripts here and uses
cmake `find_package(Python3` when running python.

Co-authored-by: Dominic Hamon <dominichamon@users.noreply.github.com>
2022-10-10 14:46:41 +03:00
rorth
12e0d70a43
Fix Solaris compilation (#1499) (#1500)
This patch fixes compilation on Solaris, addressing the problems reported
in Issue #1499:

* Provide `HOST_NAME_MAX` definition.
* Match `sysconf(3C)` return type.
* Avoid `-Wcast-qual` warnings with `libkstat(3KSTAT)` functions.
* Avoid clash with `<floatingpoint.h>` `single` typedef.
2022-10-06 09:18:55 +01:00
AJ Heller
13196fff84
Clean up test documentation formatting (#1475) 2022-08-27 20:41:33 +03:00
Dominic Hamon
974cd5a5c5
Ensure we don't need benchmark installed to pass c++ feature checks (#1456)
* Ensure we don't need benchmark installed to pass c++ feature checks

Requires removal of some dependencies on benchmark.h from internal
low-level headers, which is a good thing.

Also added better logging to the feature check cmake module.
2022-08-04 15:33:35 +01:00
Vy Nguyen
5eb16eebb3
Explicitly cast int literals to int8_t in tests to silence implicit-conversion warnings (#1455)
* Explicitly cast int literals to int8_t in tests so silence implicit-conversion warnings

Error came from:
```
: error: implicit conversion loses integer precision: 'const int' to 'const signed char' [-Werror,-Wimplicit-int-conversion]
```

* clang format

* undo deleted line
2022-08-04 09:18:19 +01:00
Vy Nguyen
1cca1d091c
Fixed build issues on window (#1449)
* Fixed build issues on window

- Added missing dlimport/export attributes in function definitions. (They are needed in both decls and defs)
- Removed dlimport/dlexprt attribute in private field. (global_context is not exported anywhere).

* fixed incorrect include path

* undo changes w.r.t HelperPrintf

* removed forward decl of private variable - instead, introduce a getter and use it.

* Removed forward decl from benchmark_gtest too

Co-authored-by: Dominic Hamon <dominichamon@users.noreply.github.com>
2022-08-03 09:44:35 +01:00
Dominic Hamon
7b3ac07517
Stop generating the export header and just check it in (#1435)
* Stop generating the export header and just check it in

* format the new header

* support windows

* format the header again

* avoid depending on internal macro

* ensure we define the right thing for windows static builds

* support older cmake

* and for tests
2022-07-20 20:34:39 +01:00
Alexander Popov
dfdda57a12
Fix DoNotOptimize() GCC compile error with some types (#1340) (#1424)
Non-const DoNotOptimize() can't compile when used with some types.
Example of code which can't compile:

char buffer3[3] = "";
benchmark::DoNotOptimize(buffer3);

Error message:

error: impossible constraint in 'asm'
  asm volatile("" : "+r"(value) : : "memory");

Introduced in 8545dfb (Fix DoNotOptimize() GCC copy overhead (#1340) (#1410))

The cause is compiler can't work with the +r constraint for types that can't
be placed perfectly in registers. For example, char array[3] can't be perfectly
fit in register on x86_64 so it requires placed in memory but constraint
doesn't allow that.

Solution
- Use +m,r constraint for the small objects so the compiler can decide to use
  register or/and memory
- For the big objects +m constraint is used which allows avoiding extra copy
  bug(see #1340)
- The same approach is used for the const version of DoNotOptimize()
  although the const version works fine with the "r" constraint only.
  Using mixed r,m constraint looks more general solution.

See
- Issue #1340 ([BUG] DoNotOptimize() adds overhead with extra copy of argument(gcc))
- Pull request #1410 (Fix DoNotOptimize() GCC copy overhead (#1340) #1410)
- Commit 8545dfb (Fix DoNotOptimize() GCC copy overhead (#1340) (#1410))
2022-07-04 10:27:05 +01:00
Dominic Hamon
b7afda2cd2
Revert "Add possibility to ask for libbenchmark version number (#1004) (#1403)" (#1417)
This reverts commit efadf67a12.
2022-06-20 17:52:03 +01:00
Alexander Popov
8545dfb3ea
Fix DoNotOptimize() GCC copy overhead (#1340) (#1410)
* Fix DoNotOptimize() GCC copy overhead (#1340)

The issue is that GCC DoNotOptimize() does a full copy of an argument
if it's not a pointer and it slows down a benchmark. If an argument is big
enough there is a memcpy() call for copying the argument. An argument
object can be a big object so DoNotOptimize() could add sufficient
overhead and affects benchmark results.

The cause is in GCC behavior with asm volatile constraints. Looks like GCC
trying to use r(register) constraint for all cases despite object size.
See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105519

The solution is the split DoNotOptimize() in two cases - value fits
in register and value doesn't fit in register. And use case specific
asm constraint. std::is_trivially_copyable trait is needed because
"+r" constraint doesn't work with non trivial copyable objects.

- Fix requires support C++11 feature std::is_trivially_copyable from GCC
  compiler. The feature has been supported since GCC 5
- Fallback for GCC version < 5 still exists but it uses "m" constraint
  which means a little bit more overhead in some cases
- Add assembly tests for issued cases

Fixes #1340

* Add supported compiler versions info for assembly tests

- Assembly tests are inherently non-portable. So explicitly add GCC
  and Clang versions required for reliable tests passed
- Write a warning message if the current compiler version isn't supported
2022-06-20 10:12:58 +01:00
Matthias Donaubauer
efadf67a12
Add possibility to ask for libbenchmark version number (#1004) (#1403)
* Add possibility to ask for libbenchmark version number (#1004)

Add a header which holds the current major, minor, and
patch number of the library. The header is auto generated
by CMake.

* Do not generate unused functions (#1004)

* Add support for version number in bazel (#1004)

* Fix clang format #1004

* Fix more clang format problems (#1004)

* Use git version feature of cmake to determine current lib version

* Rename version_config header to version

* Bake git version into bazel build

* Use same input config header as in cmake for version.h

* Adapt the releasing.md to include versioning in bazel
2022-06-20 09:45:50 +01:00
Dominic Hamon
2365c4a603
add multiple OSes to bazel workflow (#1412)
* add multiple OSes to bazel workflow

* correct indent

* only set copts when they're supported by the OS

* os check should work

* pull out cxx03_test for per-platform stuff

* attempt to fix windows test output
2022-06-13 17:45:20 +01:00
Dominic Hamon
920fa14898 fix some build warnings on type conversions 2022-06-08 10:32:20 +01:00
Matthdonau
7eb8c0fe45
Introduce warmup phase to BenchmarkRunner (#1130) (#1399)
* Introduce warmup phase to BenchmarkRunner (#1130)

In order to account for caching effects in user
benchmarks introduce a new command line option
"--benchmark_min_warmup_time"
which allows to specify an amount of time for
which the benchmark should be run before results
are meaningful.

* Adapt review suggestions regarding introduction of warmup phase (#1130)

* Fix BM_CHECK call in MinWarmUpTime (#1130)

* Fix comment on requirements of MinWarmUpTime (#1130)

* Add basic description of warmup phase mechanism to user guide (#1130)
2022-05-23 13:50:17 +01:00
Matthdonau
37be1e8252
Add option to get the verbosity provided by commandline flag -v (#1330) (#1397)
* Add option to get the verbosity provided by commandline flag -v (#1330)

* replace assert with test failure

asserts are stripped out in non debug builds, and we run tests in non-debug CI bots.

* clang-format my own tweak

Co-authored-by: Dominic Hamon <dominichamon@users.noreply.github.com>
2022-05-17 17:59:36 +01:00
Dominic Hamon
8d86026c67
Enable -Wconversion (#1390)
Requires some casts here and there, but nothing unreasonable.

Fixes #1268
2022-05-01 19:56:30 +01:00
Dominic Hamon
a162a38ca0
Filter out benchmarks that start with "DISABLED_" (#1387)
* Filter out benchmarks that start with "DISABLED_"

This could be slightly more elegant, in that the registration and the
benchmark definition names have to change.  Ideally, we'd still register
without the DISABLED_ prefix and it would all "just work".

Fixes #1365

* add some documentation
2022-05-01 10:41:34 +01:00
Vy Nguyen
eacce0b503
Add SetBenchmarkFilter() to set --benchmark_filter flag value in user code (#1362)
* Add SetBenchmarkFilter() to set --benchmark_filter flag value in user code.

Use case:  Provide an API to set this flag indepedence of the flag's implementation (ie., absl flag vs benchmark's flag facility)

* add test

* added notes on Initialize()
2022-03-08 16:02:37 +00:00
Bátor Tallér
d08e7b6056
Allow setting the default time unit globally (#1337)
* Add option to set the default time unit globally

This commit introduces the `--benchmark_time_unit={ns|us|ms|s}` command line argument. The argument only affects benchmarks where the time unit is not set explicitly.

* Update AUTHORS and CONTRIBUTORS

* Test `SetDefaultTimeUnit`

* clang format

* Use `GetDefaultTimeUnit()` for initializing `TimeUnit` variables

* Review fixes

* Export functions

* Add comment
2022-03-04 11:07:01 +00:00
Sergiu Deitsch
9e47d070fe
annotate and export public symbols (#1321) 2022-02-14 10:48:53 +00:00
Dominic Hamon
6e51dcbcc3
Expose default display reporter creation in public API (#1344)
* Expose default display reporter creation in public API

this is useful when a custom reporter wants to fall back on the default
display reporter, but doesn't necessarily have access to the benchmark
library flag configuration.

* Make use of unique_ptr in the random interleaving test.

* clang-format
2022-02-11 10:23:05 +00:00
staffantj
d2cbd4b26a
Avoid potential truncation issues for the integral type parameterized tests. (#1341)
* The parameterized tests check both floating point and integral types. We might as well use types that avoid truncation warnings across the platforms

* static_cast version of how to avoid truncation warnings in basic_test

Co-authored-by: Staffan Tjernstrom <staffantj@users.noreply.github.com>
2022-02-08 16:40:43 +00:00
Liqiang TAO
d0fbf8ac23
Cache PerfCounters instance in PerfCountersMeasurement (#1308)
This patch fixes #1306, by reducing the pinned instances of
PerfCounters.

The issue is caused by creating multiple pinned events in the
same thread, doing so results in the Snapshot(PerfCounterValues* values)
failing, and that's now discoverable.
Creating multile pinned events is an unsupported behavior currently.
The error would be detected at read() time, not
perf_event_open() / iotcl() time.

The unsupported benavior above is confirmed by Stephane Eranian @seranian,
and he also pointed the dectection method.

Finished this patch under the guidance of Mircea Trofin @mtrofin.
2022-01-25 10:14:20 +00:00
dominc8
ab867074da
clang-tidy: readability-redundant and performance (#1298)
* clang-tidy: readability-redundant-*

* clang-tidy: performance-*
2021-12-06 11:18:04 +00:00
dominc8
680d3fdbb5
Add clang-tidy check (#1290)
* Add clang-tidy.yml and .clang-tidy

* Add mention to authors/contributors

* Temp fix 2 clang-tidy issues

* Enable clang-tidy on pull requests

* Exclude gtest source files from clang-tidy
2021-11-25 15:47:44 +00:00
Dominic Hamon
88ea9d9005 lose some build warnings 2021-11-19 19:54:05 +00:00
Vy Nguyen
b5bb9f0675
Add Setup/Teardown option on Benchmark. (#1269)
* Add Setup/Teardown option on Benchmark.

Motivations:
- feature parity with our internal library. (which has ~718 callers)
- more flexible than cordinating setup/teardown inside the benchmark routine.

* change Setup/Teardown callback type to raw function pointers

* add test file to cmake file

* move b.Teardown() up

* add const to param of Setup/Teardown callbacks

* fix  comment and add doc to user_guide

* fix typo

* fix doc, fix test and add bindings to python/benchmark.cc

* fix binding again

* remove explicit C cast - that was wrong

* change policy to reference_internal

* try removing the bindinds ...

* clean up

* add more tests with repetitions and fixtures

* more comments

* init setup/teardown callbacks to NULL

* s/nullptr/NULL

* removed unused var

* change assertion on fixture_interaction::fixture_setup

* move NULL init to .cc file
2021-11-17 16:51:55 +00:00
Dominic Hamon
c07a498924
format tests with clang-format (#1282) 2021-11-10 16:22:31 +00:00
Bensuperpc
329fb06d99
Fix error with Fix Werror=old-style-cast (#1272)
* Fix Werror=old-style-cast

Signed-off-by: Bensuperpc <bensuperpc@gmail.com>

* Fix Werror=old-style-cast

Signed-off-by: Bensuperpc <bensuperpc@gmail.com>

* Fix Werror=old-style-cast

Signed-off-by: Bensuperpc <bensuperpc@gmail.com>

* Fix typo

Signed-off-by: Bensuperpc <bensuperpc@gmail.com>

* Fix build error with MacOS

Signed-off-by: Bensuperpc <bensuperpc@gmail.com>

* Revert "Fix build error with MacOS"

This reverts commit cee213bb95.
2021-11-04 12:09:10 +00:00
Bensuperpc
8826ef792f
Fix error Wshorten-64-to-32 (#1273)
Signed-off-by: Bensuperpc <bensuperpc@gmail.com>
2021-11-04 10:26:11 +00:00
Vy Nguyen
8aae0a4f67
[cleanup] Change == "" to .empty() on string to avoid clang-tidy warnings (#1271) 2021-11-03 14:54:07 +00:00
Vy Nguyen
4f31803ebb
Fix un-initted error in test and fix change the API previously proposed to use std::string instead of raw char* (#1266)
* Fix un-initted error in test.

Found by -Werror,-Wsometimes-uninitialized

* Update spec_arg_test.cc

* additional change:
- Change the API on GetBenchmarkFilter and the `spec` to std::string because google C++ styleguide internally kind of discouraged  using raw const char*
2021-10-29 11:48:56 +01:00
Vy Nguyen
4f47ed2c9a
[RFC] Adding API for setting/getting benchmark_filter flag? (#1254)
* [RFC] Adding API for setting/getting benchmark_filter flag?

This PR is more of a Request-for-comment - open to other ideas/suggestions as well.

Details:
This flag has different implementations(absl vs benchmark) and since the proposal to add absl as a dependency was rejected, it would be nice to have a reliable (and less hacky) way to access this flag internally.
(Actually, reading it isn't much a problem but setting it is).

Internally, we have a sizeable number users to use absl::SetFlags to set this flag. This will not work with benchmark-flags.

Another motivation is that not all users use the command line flag. Some prefer to programmatically set this value.

* fixed build errors

* fix lints again

* per discussion: add additional RunSpecifiedBenchmarks instead.

* add tests

* fix up tests

* clarify comment

* fix stray : in test

* more assertion in test

* add test file to test/CMakeLists.txt

* more test

* make test ISO C++ compliant

* fix up BUILD file to pass the flag
2021-10-27 08:52:57 +01:00
Vy Nguyen
fca348296f
Allow template arguments to be specified directly on the BENCHMARK macro (#1262)
* Allow template arguments to be specifed directly on the BENCHMARK macro/

Use cases:
 - more convenient (than having to use a separate BENCHMARK_TEMPLATE)
 - feature parity with our internal library.

* fix tests

* updated docs
2021-10-26 15:38:12 +01:00
Byoungchan Lee
80d70ddd94
Fix -Wdeprecated-declarations warning once more. (#1256)
In #1238, one of MemoryManager's Stop methods was marked as deprecated
and this method is used in the same header. This change generated
-Wdeprecated-declarations warning on every file that includes
"benchmark.h". Use gcc's diagnostics to fix this warning.
2021-10-21 10:10:38 +01:00
Byoungchan Lee
f730846b0a
Fix -Wdeprecated-declarations warning triggered by clang-cl. (#1245)
WebRTC uses Google Benchmarks as a dependency and uses Chromium's build
infrastructure. Chromium is compiled using clang-cl on Windows, and the
-Wdeprecated-declarations warning is triggered. Because clang-cl accepts
gcc's diagnostic prama and defines the __clang__ macro,
using it can solve this issue.

Bug: webrtc:13280
2021-10-18 11:31:51 +01:00
Dominic Hamon
c23a001252
Remove unused parameter from lambda. (#1223)
Fixes #1222
2021-09-07 17:54:23 +01:00
Roman Lebedev
4f8070590c
COnsole reporter: if statistic produces percents, format it as such (#1221) 2021-09-06 11:33:27 +03:00
Roman Lebedev
45b194e4d4
Introduce Coefficient of variation aggregate (#1220)
* Introduce Coefficient of variation aggregate

I believe, it is much more useful / use to understand,
because it is already normalized by the mean,
so it is not affected by the duration of the benchmark,
unlike the standard deviation.

Example of real-world output:
```
raw.pixls.us-unique/GoPro/HERO6 Black$ ~/rawspeed/build-old/src/utilities/rsbench/rsbench GOPR9172.GPR --benchmark_repetitions=27 --benchmark_display_aggregates_only=true --benchmark_counters_tabular=true
2021-09-03T18:05:56+03:00
Running /home/lebedevri/rawspeed/build-old/src/utilities/rsbench/rsbench
Run on (32 X 3596.16 MHz CPU s)
CPU Caches:
  L1 Data 32 KiB (x16)
  L1 Instruction 32 KiB (x16)
  L2 Unified 512 KiB (x16)
  L3 Unified 32768 KiB (x2)
Load Average: 7.00, 2.99, 1.85
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Benchmark                                                      Time             CPU   Iterations  CPUTime,s CPUTime/WallTime     Pixels Pixels/CPUTime Pixels/WallTime Raws/CPUTime Raws/WallTime WallTime,s
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
GOPR9172.GPR/threads:32/process_time/real_time_mean         11.1 ms          353 ms           27   0.353122          31.9473        12M       33.9879M        1085.84M      2.83232       90.4864  0.0110535
GOPR9172.GPR/threads:32/process_time/real_time_median       11.0 ms          352 ms           27   0.351696          31.9599        12M       34.1203M        1090.11M      2.84336       90.8425  0.0110081
GOPR9172.GPR/threads:32/process_time/real_time_stddev      0.159 ms         4.60 ms           27   4.59539m        0.0462064          0       426.371k        14.9631M    0.0355309       1.24692   158.944u
GOPR9172.GPR/threads:32/process_time/real_time_cv           1.44 %          1.30 %            27  0.0130136         1.44633m          0      0.0125448       0.0137802    0.0125448     0.0137802  0.0143795
```

Fixes https://github.com/google/benchmark/issues/1146

* Be consistent, it's CV, not 'rel std dev'
2021-09-03 18:44:10 +01:00
Roman Lebedev
12dc5eeafc
Statistics: add support for percentage unit in addition to time (#1219)
* Statistics: add support for percentage unit in addition to time

I think, `stddev` statistic is useful, but confusing.

What does it mean if `stddev` of `1ms` is reported?
Is that good or bad? If the `median` is `1s`,
then that means that the measurements are pretty noise-less.

And what about `stddev` of `100ms` is reported?
If the `median` is `1s` - awful, if the `median` is `10s` - good.

And hurray, there is just the statistic that we need:
https://en.wikipedia.org/wiki/Coefficient_of_variation

But, naturally, that produces a value in percents,
but the statistics are currently hardcoded to produce time.

So this refactors thinkgs a bit, and allows a percentage unit for statistics.

I'm not sure whether or not `benchmark` would be okay
with adding this `RSD` statistic by default,
but regales, that is a separate patch.

Refs. https://github.com/google/benchmark/issues/1146

* Address review notes
2021-09-03 15:36:56 +01:00
Vy Nguyen
dc1a97174d
Introduce accessors for currently public data members (threads and thread_index) (#1208)
* [benchmark] Introduce accessors for currently public data members `threads` and `thread_index`

Also deprecate the direct access to these fields.

Motivations:

Our internal library provides accessors for those fields because the styleguide disalows accessing classes' data members directly (even if they're const).
There has been a discussion to simply move internal library to make its fields public similarly to the OSS version here, however, the concern is that these kinds of direct access would prevent many types of future design changes (eg how/whether the values would be stored in the data member)

I think the concensus in the end is that we'd change the external library for this case.
AFAIK, there are three important third_party users that we'd need to migrate: tcmalloc, abseil and tensorflow.
Please let me know if I'm missing anyone else.

* [benchmark] Introduce accessors for currently public data members `threads` and `thread_index`

Also deprecate the direct access to these fields.

Motivations:

Our internal library provides accessors for those fields because the styleguide disalows accessing classes' data members directly (even if they're const).
There has been a discussion to simply move internal library to make its fields public similarly to the OSS version here, however, the concern is that these kinds of direct access would prevent many types of future design changes (eg how/whether the values would be stored in the data member)

I think the concensus in the end is that we'd change the external library for this case.
AFAIK, there are three important third_party users that we'd need to migrate: tcmalloc, abseil and tensorflow.
Please let me know if I'm missing anyone else.

* [benchmark] Introduce accessors for currently public data members `threads` and `thread_index`

Also deprecate direct access to `.thread_index` and make threads a private field

Motivations:

Our internal library provides accessors for those fields because the styleguide disalows accessing classes' data members directly (even if they're const).
There has been a discussion to simply move internal library to make its fields public similarly to the OSS version here, however, the concern is that these kinds of direct access would prevent many types of future design changes (eg how/whether the values would be stored in the data member)

I think the concensus in the end is that we'd change the external library for this case.
AFAIK, there are three important third_party users that we'd need to migrate: tcmalloc, abseil and tensorflow.
Please let me know if I'm missing anyone else.

* [benchmark] Introduce accessors for currently public data members `threads` and `thread_index`

Also deprecate direct access to `.thread_index` and make threads a private field

Motivations:

Our internal library provides accessors for those fields because the styleguide disalows accessing classes' data members directly (even if they're const).
There has been a discussion to simply move internal library to make its fields public similarly to the OSS version here, however, the concern is that these kinds of direct access would prevent many types of future design changes (eg how/whether the values would be stored in the data member)

I think the concensus in the end is that we'd change the external library for this case.
AFAIK, there are three important third_party users that we'd need to migrate: tcmalloc, abseil and tensorflow.
Please let me know if I'm missing anyone else.

* [benchmark] Introduce accessors for currently public data members `threads` and `thread_index`

Also deprecate direct access to `.thread_index` and make threads a private field

Motivations:

Our internal library provides accessors for those fields because the styleguide disalows accessing classes' data members directly (even if they're const).
There has been a discussion to simply move internal library to make its fields public similarly to the OSS version here, however, the concern is that these kinds of direct access would prevent many types of future design changes (eg how/whether the values would be stored in the data member)

I think the concensus in the end is that we'd change the external library for this case.
AFAIK, there are three important third_party users that we'd need to migrate: tcmalloc, abseil and tensorflow.
Please let me know if I'm missing anyone else.

* [benchmark] Introduce accessors for currently public data members `threads` and `thread_index`

Also deprecate direct access to `.thread_index` and make threads a private field

Motivations:

Our internal library provides accessors for those fields because the styleguide disalows accessing classes' data members directly (even if they're const).
There has been a discussion to simply move internal library to make its fields public similarly to the OSS version here, however, the concern is that these kinds of direct access would prevent many types of future design changes (eg how/whether the values would be stored in the data member)

I think the concensus in the end is that we'd change the external library for this case.
AFAIK, there are three important third_party users that we'd need to migrate: tcmalloc, abseil and tensorflow.
Please let me know if I'm missing anyone else.

* [benchmark] Introduce accessors for currently public data members `threads` and `thread_index`

Also deprecate direct access to `.thread_index` and make threads a private field

Motivations:

Our internal library provides accessors for those fields because the styleguide disalows accessing classes' data members directly (even if they're const).
There has been a discussion to simply move internal library to make its fields public similarly to the OSS version here, however, the concern is that these kinds of direct access would prevent many types of future design changes (eg how/whether the values would be stored in the data member)

I think the concensus in the end is that we'd change the external library for this case.
AFAIK, there are three important third_party users that we'd need to migrate: tcmalloc, abseil and tensorflow.
Please let me know if I'm missing anyone else.

* [benchmark] Introduce accessors for currently public data members `threads` and `thread_index`

Also deprecate direct access to `.thread_index` and make threads a private field

Motivations:

Our internal library provides accessors for those fields because the styleguide disalows accessing classes' data members directly (even if they're const).
There has been a discussion to simply move internal library to make its fields public similarly to the OSS version here, however, the concern is that these kinds of direct access would prevent many types of future design changes (eg how/whether the values would be stored in the data member)

I think the concensus in the end is that we'd change the external library for this case.
AFAIK, there are three important third_party users that we'd need to migrate: tcmalloc, abseil and tensorflow.
Please let me know if I'm missing anyone else.

* [benchmark] Introduce accessors for currently public data members `threads` and `thread_index`

Also deprecate direct access to `.thread_index` and make threads a private field

Motivations:

Our internal library provides accessors for those fields because the styleguide disalows accessing classes' data members directly (even if they're const).
There has been a discussion to simply move internal library to make its fields public similarly to the OSS version here, however, the concern is that these kinds of direct access would prevent many types of future design changes (eg how/whether the values would be stored in the data member)

I think the concensus in the end is that we'd change the external library for this case.
AFAIK, there are three important third_party users that we'd need to migrate: tcmalloc, abseil and tensorflow.
Please let me know if I'm missing anyone else.

* [benchmark] Introduce accessors for currently public data members `threads` and `thread_index`

Also deprecate direct access to `.thread_index` and make threads a private field

Motivations:

Our internal library provides accessors for those fields because the styleguide disalows accessing classes' data members directly (even if they're const).
There has been a discussion to simply move internal library to make its fields public similarly to the OSS version here, however, the concern is that these kinds of direct access would prevent many types of future design changes (eg how/whether the values would be stored in the data member)

I think the concensus in the end is that we'd change the external library for this case.
AFAIK, there are three important third_party users that we'd need to migrate: tcmalloc, abseil and tensorflow.
Please let me know if I'm missing anyone else.
2021-08-23 09:06:57 +01:00
Dominic Hamon
19026e232c
fix clang-tidy warnings (#1195) 2021-06-29 11:06:53 +01:00
Dominic Hamon
1799e1b9ec
prefix VLOG (#1187) 2021-06-24 18:55:37 +01:00
Dominic Hamon
6a5bf081d3
prefix macros to avoid clashes (#1186) 2021-06-24 18:21:59 +01:00