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.

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

Optimize AtomicBool::fetch_nand #41143

Merged
merged 2 commits into from Apr 8, 2017
Merged

Optimize AtomicBool::fetch_nand #41143

merged 2 commits into from Apr 8, 2017

Conversation

ghost
Copy link

@ghost ghost commented Apr 7, 2017

This is an attempt to push the PR #40563 to completion.

Benchmark: source
Improvement:

 name  old_ ns/iter  new_ce_ ns/iter  diff ns/iter   diff % 
 1t    146,440       89,904                -56,536  -38.61% 
 2t    561,456       316,835              -244,621  -43.57% 
 4t    2,822,821     1,005,424          -1,817,397  -64.38% 

r? @eddyb
cc @alexcrichton @nagisa

@nagisa
Copy link
Member

nagisa commented Apr 7, 2017

The difference between swap and compare_exchange in previous benchmark seemed like something that would be within error, with swap implementation being significantly easier to comprehend.

I would r+ if CAS was replaced with swap.

@ghost
Copy link
Author

ghost commented Apr 7, 2017

@nagisa Fair enough. Replaced CAS with swap.

Yeah, CAS and swap are equally performant according to the benchmarks.

@eddyb
Copy link
Member

eddyb commented Apr 7, 2017

@bors r=nagisa

@bors
Copy link
Contributor

bors commented Apr 7, 2017

📌 Commit f7ffe5b has been approved by nagisa

@bors
Copy link
Contributor

bors commented Apr 7, 2017

⌛ Testing commit f7ffe5b with merge df15b21...

@bors
Copy link
Contributor

bors commented Apr 7, 2017

💔 Test failed - status-appveyor

@TimNN
Copy link
Contributor

TimNN commented Apr 7, 2017

@bors retry

@bors
Copy link
Contributor

bors commented Apr 7, 2017

⌛ Testing commit f7ffe5b with merge 9b29afe...

@bors
Copy link
Contributor

bors commented Apr 7, 2017

💔 Test failed - status-appveyor

@TimNN
Copy link
Contributor

TimNN commented Apr 7, 2017

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Apr 7, 2017
…r=nagisa

Optimize AtomicBool::fetch_nand

This is an attempt to push the PR rust-lang#40563 to completion.

Benchmark: [source](https://gist.github.com/stjepang/023f5025623f5474184f9f4dfd6379ae)
Improvement:

```
 name  old_ ns/iter  new_ce_ ns/iter  diff ns/iter   diff %
 1t    146,440       89,904                -56,536  -38.61%
 2t    561,456       316,835              -244,621  -43.57%
 4t    2,822,821     1,005,424          -1,817,397  -64.38%
```

r? @eddyb
cc @alexcrichton @nagisa
bors added a commit that referenced this pull request Apr 8, 2017
Rollup of 3 pull requests

- Successful merges: #41135, #41143, #41146
- Failed merges:
@bors
Copy link
Contributor

bors commented Apr 8, 2017

⌛ Testing commit f7ffe5b with merge a05813d...

@bors
Copy link
Contributor

bors commented Apr 8, 2017

💔 Test failed - status-appveyor

@TimNN
Copy link
Contributor

TimNN commented Apr 8, 2017

TimNN added a commit to TimNN/rust that referenced this pull request Apr 8, 2017
…r=nagisa

Optimize AtomicBool::fetch_nand

This is an attempt to push the PR rust-lang#40563 to completion.

Benchmark: [source](https://gist.github.com/stjepang/023f5025623f5474184f9f4dfd6379ae)
Improvement:

```
 name  old_ ns/iter  new_ce_ ns/iter  diff ns/iter   diff %
 1t    146,440       89,904                -56,536  -38.61%
 2t    561,456       316,835              -244,621  -43.57%
 4t    2,822,821     1,005,424          -1,817,397  -64.38%
```

r? @eddyb
cc @alexcrichton @nagisa
@TimNN TimNN mentioned this pull request Apr 8, 2017
bors added a commit that referenced this pull request Apr 8, 2017
Rollup of 4 pull requests

- Successful merges: #41135, #41143, #41146, #41152
- Failed merges:
@bors bors merged commit f7ffe5b into rust-lang:master Apr 8, 2017
@ghost ghost deleted the optimize-bool-fetch-nand branch April 8, 2017 09:57
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.

4 participants