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.

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

Add hardware supported popcount #26

Merged
merged 1 commit into from Sep 2, 2018

Conversation

Projects
None yet
2 participants
@Izaron
Copy link
Contributor

Izaron commented Aug 23, 2018

Related issue - #25

@codecov

This comment has been minimized.

Copy link

codecov bot commented Aug 23, 2018

Codecov Report

Merging #26 into develop will increase coverage by 0.44%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #26      +/-   ##
===========================================
+ Coverage    75.41%   75.86%   +0.44%     
===========================================
  Files            5        5              
  Lines          541      551      +10     
  Branches       198      199       +1     
===========================================
+ Hits           408      418      +10     
  Misses          23       23              
  Partials       110      110
Impacted Files Coverage Δ
include/boost/dynamic_bitset/dynamic_bitset.hpp 74.75% <ø> (ø) ⬆️
include/boost/detail/dynamic_bitset.hpp 93.54% <100%> (+3.07%) ⬆️

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 b944aa3...a87cbee. Read the comment docs.

@jeking3
Copy link
Collaborator

jeking3 left a comment

Would probably run this in the debugger to satisfy myself that it's actually making it into the template specializations you added... but it does pass CI which is a good thing. It's too bad there's no standard for this...

@Izaron

This comment has been minimized.

Copy link
Contributor

Izaron commented Aug 30, 2018

@jeking3 Thank you! Actually we started to discuss some "standardization" of intrinsics/"smart"/fast bit algorithms and their problems in the boost mailing list - the thread is here. It could be part of an existing library, probably Boost.Integer.

The main problem for me is testing the intrinsics code - though there are only two major intrinsic versions (MSVC/Gcc or Clang), it apparently requires a heap of possible environments of different systems, compilers, their versions... I didn't manage to get Appveyor to work, but you may look at Travis outcome - click on a job and scroll down to see differences between naive algos and smart ones on a handcrafted benchmark.

@jeking3

This comment has been minimized.

Copy link
Collaborator

jeking3 commented Aug 31, 2018

Curious if you have any benchmarks showing the performance improvement of the changes?

@Izaron

This comment has been minimized.

Copy link
Contributor

Izaron commented Sep 1, 2018

Yes I do, I wrote a benchmark using this library, there is source code, old performance, new. It does nothing on small types, slightly speeds up int and rushes forward on longs.
I honestly didn't test the performance on Windows, I just guess that it should show the same outcome.
(P.S. Changed the PR a bit - just added some copyright line about me...)

@Izaron Izaron force-pushed the Izaron:popcount branch from 607cb1f to a87cbee Sep 1, 2018

@jeking3

This comment has been minimized.

Copy link
Collaborator

jeking3 commented Sep 2, 2018

Nice, so performance improvement on 64-bit is 2.375x (3.8 / 1.6). Thanks.

@jeking3 jeking3 merged commit a90fe08 into boostorg:develop Sep 2, 2018

4 checks passed

codecov/patch 100% of diff hit (target 75.41%)
Details
codecov/project 75.86% (+0.44%) compared to b944aa3
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment