Skip to content

atomic64: Use AtomicUsize on 64-bit targets #82

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

Closed
wants to merge 3 commits into from

Conversation

kamalmarhubi
Copy link
Contributor

@kamalmarhubi kamalmarhubi commented Nov 4, 2016

This builds on the work in #64 to allow stable Rust users to get atomics
if they run on 64-bit architectures.

Benchmark improvements:

name                                                old ns/iter  new ns/iter  diff ns/iter   diff %
counter::bench_counter_no_labels                    49           16                    -33  -67.35%
counter::bench_counter_with_label_values            195          139                   -56  -28.72%
counter::bench_counter_with_mapped_labels           715          371                  -344  -48.11%
counter::bench_counter_with_prepared_mapped_labels  436          219                  -217  -49.77%
gauge::bench_gauge_no_labels                        48           15                    -33  -68.75%
gauge::bench_gauge_with_label_values                167          115                   -52  -31.14%
histogram::bench_histogram_no_labels                179          30                   -149  -83.24%
histogram::bench_histogram_timer                    276          138                  -138  -50.00%
histogram::bench_histogram_with_label_values        363          137                  -226  -62.26%

Benchmarks run with default features (no "nightly" feature) on
nightly-2016-09-22, which corresponds roughly to 1.12.0.

Tested:
Ran tests on the following combinations (all unknown-linux-gnu):
- stable Rust x86_64
- stable Rust i686
- nightly Rust x86_64
- nightly Rust i686
- nightly Rust x86_64 with --features=nightly
- nightly Rust i686 with --features=nightly

@kamalmarhubi
Copy link
Contributor Author

CI should be extended to cover some of these combinations. Probably ok in a later PR?

@kamalmarhubi
Copy link
Contributor Author

Test failure is unrelated. Fixed in #83; can rebase after that's merged.

Copy link
Member

@overvenus overvenus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kamalmarhubi Thank you very much! There will be a huge performance leap for most stable build. 👍

@siddontang
Copy link
Contributor

Thanks @kamalmarhubi

Could you merge the master and fix the CI ?

Preparation for refactoring to separate modules for each implementation.
This commit will make later diffs easier to read.
Conditionally use the relevant file to separate out the different
implementations.
This builds on the work in tikv#64 to allow stable Rust users to get atomics
if they run on 64-bit architectures.

Benchmark improvements:

    name                                                old ns/iter  new ns/iter  diff ns/iter   diff %
    counter::bench_counter_no_labels                    49           16                    -33  -67.35%
    counter::bench_counter_with_label_values            195          139                   -56  -28.72%
    counter::bench_counter_with_mapped_labels           715          371                  -344  -48.11%
    counter::bench_counter_with_prepared_mapped_labels  436          219                  -217  -49.77%
    gauge::bench_gauge_no_labels                        48           15                    -33  -68.75%
    gauge::bench_gauge_with_label_values                167          115                   -52  -31.14%
    histogram::bench_histogram_no_labels                179          30                   -149  -83.24%
    histogram::bench_histogram_timer                    276          138                  -138  -50.00%
    histogram::bench_histogram_with_label_values        363          137                  -226  -62.26%

Benchmarks run with default features (no "nightly" feature) on
nightly-2016-09-22, which corresponds roughly to 1.12.0.

Tested:
  Ran tests on the following combinations (all unknown-linux-gnu):
    - stable Rust x86_64
    - stable Rust i686
    - nightly Rust x86_64
    - nightly Rust i686
    - nightly Rust x86_64 with `--features=nightly`
    - nightly Rust i686 with `--features=nightly`
@kamalmarhubi
Copy link
Contributor Author

I rebased this to pick up the lint fix in #83. But don't merge this until I send up the CI improvements (wip, will be a separate PR). I'll rebase this again after that is merged.

@siddontang
Copy link
Contributor

LGTM

@siddontang
Copy link
Contributor

PTAL @BusyJay

@kamalmarhubi
Copy link
Contributor Author

Just to make it clear this shouldn't be merged till after the CI improvements, I'm closing the PR for now. Will reopen hopefully in the next day or two.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants