Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test: update nanobench from release 4.0.0 to 4.3.4 #22082

Merged
merged 2 commits into from
Jun 2, 2021
Merged

test: update nanobench from release 4.0.0 to 4.3.4 #22082

merged 2 commits into from
Jun 2, 2021

Conversation

martinus
Copy link
Contributor

@martinus martinus commented May 27, 2021 •
edited
Loading

This updates the third-party library nanobench with the latest release. It contains mostly minor bugfixes, a new pyperf output format, ability to suppress warnings with environment variable NANOBENCH_SUPPRESS_WARNINGS. Full changelog:

v4.0.2

  • Changed doNotOptimizeAway to what google benchmark is doing. The old code did not work on some machines.
  • fix: display correct "total" value
  • minor Documentation updates

v4.1.0

v4.2.0

v4.3.0

  • ankerl::nanobench::Rng can now return the state with std::vector<uint64_t> Rng::state(), and this can also be used to initialize the Rng.

v4.3.1

  • Minor cmake improvements when integrationg as a third-party library: add alias nanobench::nanobench, default to C++17

v4.3.2

  • Fixed a MSVC 2015 build problem
  • updates license to 2021.
  • build should now work with very old linux headers
  • Also disable UBSAN (bitcoin needed to add a suppression)

v4.3.3

  • Do not use locale-dependent std::to_string

v4.3.4

  • Add missing sanitizer suppression to rotl

@martinus
Copy link
Contributor Author

This update should also fix #21549

@sipa
Copy link
Member

sipa commented May 27, 2021

Should we consider turning the nanobench import into a git-subtree?

@martinus martinus force-pushed the 2021-05-update-nanobench branch from 2e17c6d to d728cfe Compare May 27, 2021 06:43
@martinus martinus changed the title test: update nanobench from release 4.0.0 to 4.3.2 test: update nanobench from release 4.0.0 to 4.3.3 May 27, 2021
@martinus
Copy link
Contributor Author

Should we consider turning the nanobench import into a git-subtree?

This sounds like a good idea. I've never used this feature though, is this something that I can do in the PR, or is this something only a maintainer can do?

@laanwj
Copy link
Member

laanwj commented May 27, 2021

Concept ACK

Should we consider turning the nanobench import into a git-subtree?

Not opposed to it, but I'm not sure adding a subtree for what is one single file makes that much sense. Now at least we don't have to carry any additional files that might be in the repository. E.g. we don't subtree tinyformat either.

@practicalswift
Copy link
Contributor

Concept ACK

@fanquake
Copy link
Member

Concept ACK - I agree that adding a subtree for a single header seems like overkill. Updates to this file would also seem to be very infrequent.

@martinus
Copy link
Contributor Author

Updates to this file would also seem to be very infrequent.

I mostly do minor bugfixes now and then when they arise, but I'd consider nanobench to be finished. So there won't be any major updates.

@theStack
Copy link
Contributor

Concept ACK

@maflcko
Copy link
Member

maflcko commented May 31, 2021

Also disable UBSAN (bitcoin needed to add a suppression)

Does that mean we can remove the suppression?

@martinus
Copy link
Contributor Author

martinus commented Jun 1, 2021

Does that mean we can remove the suppression?

It should work, but I didn't try it in the bitcoin repository. Also I don't have clang12 (yet). Should I add a commit to remove the suppression?

@maflcko maflcko mentioned this pull request Jun 1, 2021
Closed
@maflcko
Copy link
Member

maflcko commented Jun 1, 2021

Yes, CI should check it

@maflcko
Copy link
Member

maflcko commented Jun 1, 2021

ci didn't like that. maybe the annotation needs to be on rotl itself?

@martinus
Copy link
Contributor Author

martinus commented Jun 1, 2021

ci didn't like that. maybe the annotation needs to be on rotl itself?

yes, it needs to be there too... its the code for a fast random generator, so overflows are expected.

martinus added 2 commits June 1, 2021 12:00
@martinus
test: update nanobench from release 4.0.0 to 4.3.4
e3c866e 
This updates the third-party library nanobench with the latest release. It contains mostly minor bugfixes, a new pyperf output format, ability to suppress warnings with environment variable `NANOBENCH_SUPPRESS_WARNINGS`. Full changelog:

v4.0.2
* Changed `doNotOptimizeAway` to what google benchmark is doing. The old code did not work on some machines.
* fix: display correct "total" value
* minor Documentation updates

v4.1.0
* Updated link to new pyperf home
* Adds ability to configure console output time unit
*  Add support for environment variable `NANOBENCH_SUPPRESS_WARNINGS`
* Nanobench is now usable with CMake's FetchContent (see documentation: https://nanobench.ankerl.com/tutorial.html#cmake-integration)

v4.2.0
* Ability to store and later compare results added, through `pyperf`.
* See https://nanobench.ankerl.com/tutorial.html#pyperf-python-pyperf-module-output
* Added lots of build targets to travis, similar to bitcoin's build.
* Some minor API & documentation improvements

v4.3.0
* `ankerl::nanobench::Rng` can now return the state with `std::vector<uint64_t> Rng::state()`, and this can also be used to initialize the Rng.

v4.3.1
* Minor cmake improvements when integrationg as a third-party library: add alias `nanobench::nanobench`, default to C++17

v4.3.2
* Fixed a MSVC 2015 build problem
* updates license to 2021.
* build should now work with very old linux headers
* Also disable UBSAN (bitcoin needed to add a suppression)

v4.3.3
* Do not use locale-dependent `std::to_string`

v4.3.4
* Add missing sanitizer suppression to `rotl`
@martinus
test: remove sanitizer suppression for nanobench
44d05d0 
In bitcoin#21738 an ASAN suppression for nanobench was added. This is not needed any more,as nanobench.h already includes the necessary suppressions for the relevant code.
@martinus martinus force-pushed the 2021-05-update-nanobench branch from 9a78ce4 to 44d05d0 Compare June 1, 2021 10:01
@martinus martinus changed the title test: update nanobench from release 4.0.0 to 4.3.3 test: update nanobench from release 4.0.0 to 4.3.4 Jun 1, 2021
@martinus
Copy link
Contributor Author

martinus commented Jun 1, 2021

I've created a new nanobench release for the missing annotation, then rebased with this version, now it compiles with the removed suppression. Ready for review!

@maflcko
Copy link
Member

maflcko commented Jun 2, 2021

review ACK 44d05d0

@maflcko maflcko merged commit 2fccd9c into bitcoin:master Jun 2, 2021
@martinus martinus deleted the 2021-05-update-nanobench branch June 2, 2021 11:24
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jun 3, 2021
Merge bitcoin#22082: test: update nanobench from release 4.0.0 to 4.3.4
ab725fb
@martinus martinus mentioned this pull request Sep 21, 2021
Closed
gwillen pushed a commit to ElementsProject/elements that referenced this pull request Jun 1, 2022
@apoelstra
Merge 2fccd9c into merged_master (Bitcoin PR bitcoin/bitcoin#22082)
e0e20ba
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
Tests Upstream
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@martinus @sipa @laanwj @practicalswift @fanquake @theStack @maflcko