boostorg / unordered

Boost.org unordered module
http://boost.org/libs/unordered
Boost Software License 1.0
61 stars 55 forks source link

Feature/foa fast iteration #187

Closed joaquintides closed 1 year ago

joaquintides commented 1 year ago

Changed iterator increment code for "regular layout" group implementations (basically, those for SSE2 and Neon). Iteration speed improves dramatically, look for "Running erasure" in:

https://github.com/boostorg/boost_unordered_benchmarks/commit/46129697b05fab67c42cfa2b918a6e40af26d6e6

cppalliance-bot commented 1 year ago

An automated preview of the documentation is available at https://187.unordered.prtest.cppalliance.org/libs/unordered/doc/html/unordered.html

pdimov commented 1 year ago

Looks good to me, although I'm not sure we should be merging to develop now. Maybe wait for the beta to go out in case there are critical fixes we need to apply.

joaquintides commented 1 year ago

The release notes added in the PR target 1.83, I just submitted it because it's ready but we can wait of course.

cppalliance-bot commented 1 year ago

An automated preview of the documentation is available at https://187.unordered.prtest.cppalliance.org/libs/unordered/doc/html/unordered.html

cppalliance-bot commented 1 year ago

An automated preview of the documentation is available at https://187.unordered.prtest.cppalliance.org/libs/unordered/doc/html/unordered.html

codecov[bot] commented 1 year ago

Codecov Report

Merging #187 (1c5640c) into develop (9aedb95) will increase coverage by 0.00%. The diff coverage is 100.00%.

Additional details and impacted files [![Impacted file tree graph](https://codecov.io/gh/boostorg/unordered/pull/187/graphs/tree.svg?width=650&height=150&src=pr&token=ZqRPZlJZ5N&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg)](https://codecov.io/gh/boostorg/unordered/pull/187?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg) ```diff @@ Coverage Diff @@ ## develop #187 +/- ## ======================================== Coverage 97.85% 97.85% ======================================== Files 91 91 Lines 13849 13875 +26 ======================================== + Hits 13552 13578 +26 Misses 297 297 ``` | [Impacted Files](https://codecov.io/gh/boostorg/unordered/pull/187?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg) | Coverage Δ | | |---|---|---| | [include/boost/unordered/detail/foa.hpp](https://codecov.io/gh/boostorg/unordered/pull/187?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg#diff-aW5jbHVkZS9ib29zdC91bm9yZGVyZWQvZGV0YWlsL2ZvYS5ocHA=) | `99.84% <100.00%> (+<0.01%)` | :arrow_up: | ------ [Continue to review full report at Codecov](https://codecov.io/gh/boostorg/unordered/pull/187?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/boostorg/unordered/pull/187?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg). Last update [9aedb95...1c5640c](https://codecov.io/gh/boostorg/unordered/pull/187?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg).
joaquintides commented 1 year ago

Latest commit (1c5640cfbe84213b9d2a7eff68e2db744cfa3817) didn't affect performance: https://github.com/boostorg/boost_unordered_benchmarks/commit/90e178376ffefe73d3e6fc7fa24ef70a26d429a6

cmazakas commented 1 year ago

Latest commit (1c5640c) didn't affect performance: boostorg/boost_unordered_benchmarks@90e1783

Well, my logic was more about the code self-documenting the assumptions.

It wasn't clear to me why we needed the null pointer check before calling ++it.

cmazakas commented 1 year ago

@pdimov @joaquintides

Now that the Boost beta is out, should we feel free to go and merge this PR?

joaquintides commented 1 year ago

FWIW, this feature is already integrated in feature/cfoa, so we can either drop this PR or merge and rebase that branch.

pdimov commented 1 year ago

We can merge it to develop, I suppose, just not to master.