Skip to content

Optimize insertion sort - #40807

Merged
bors merged 1 commit into
masterfrom
unknown repository
Mar 26, 2017
Merged

Optimize insertion sort#40807
bors merged 1 commit into
masterfrom
unknown repository

Conversation

@ghost

@ghostghost commented Mar 24, 2017

Copy link
Copy Markdown

This change slightly changes the main iteration loop so that LLVM can optimize it more efficiently.

Benchmark:

name before ns/iter after ns/iter diff ns/iter diff %
slice::sort_unstable_small_ascending 39 (2051 MB/s) 38 (2105 MB/s) -1 -2.56%
slice::sort_unstable_small_big_random 579 (2210 MB/s) 575 (2226 MB/s) -4 -0.69%
slice::sort_unstable_small_descending 80 (1000 MB/s) 70 (1142 MB/s) -10 -12.50%
slice::sort_unstable_small_random 396 (202 MB/s) 386 -10 -2.53%

The benchmark is not a fluke. I can see that performance on small_descending is consistently better after this change. I'm not 100% sure why this makes things faster, but my guess would be that v.len()+1 to the compiler looks like it could in theory overflow.

This change slightly changes the main iteration loop so that LLVM can
optimize it more efficiently.
Benchmark:
name before ns/iter after ns/iter diff ns/iter diff %
slice::sort_unstable_small_ascending 39 (2051 MB/s) 38 (2105 MB/s) -1 -2.56%
slice::sort_unstable_small_big_random 579 (2210 MB/s) 575 (2226 MB/s) -4 -0.69%
slice::sort_unstable_small_descending 80 (1000 MB/s) 70 (1142 MB/s) -10 -12.50%
slice::sort_unstable_small_random 396 (202 MB/s) 386 -10 -2.53%
@rust-highfive

Copy link
Copy Markdown
Contributor

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

@ghost

Copy link
Copy Markdown
Author

r? @alexcrichton

@alexcrichton

Copy link
Copy Markdown
Member

@bors: r+

@bors

bors commented Mar 25, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit 2c816f7 has been approved by alexcrichton

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Mar 25, 2017
…=alexcrichton
Optimize insertion sort
This change slightly changes the main iteration loop so that LLVM can optimize it more efficiently.
Benchmark:
```
name before ns/iter after ns/iter diff ns/iter diff %
slice::sort_unstable_small_ascending 39 (2051 MB/s) 38 (2105 MB/s) -1 -2.56%
slice::sort_unstable_small_big_random 579 (2210 MB/s) 575 (2226 MB/s) -4 -0.69%
slice::sort_unstable_small_descending 80 (1000 MB/s) 70 (1142 MB/s) -10 -12.50%
slice::sort_unstable_small_random 396 (202 MB/s) 386 -10 -2.53%
```
The benchmark is not a fluke. I can see that performance on `small_descending` is consistently better after this change. I'm not 100% sure why this makes things faster, but my guess would be that `v.len()+1` to the compiler looks like it could in theory overflow.
bors added a commit that referenced this pull request Mar 25, 2017
bors added a commit that referenced this pull request Mar 25, 2017
@nagisa

Copy link
Copy Markdown
Member

Since this is internal to libstd/core, could you check whether the inclusive range syntax makes things better as well?

i.e. 2 ... v.len()

@ghost

Copy link
Copy Markdown
Author

@nagisa Inclusive range syntax makes performance slightly worse, actually...

With the old insertion sort and with inclusive range syntax there's a bound check at the beginning of insertion sort. This PR removes the bound check.

If you want to play with this, here's a playpen link.

@bors

bors commented Mar 25, 2017

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 2c816f7 with merge 04e47d7...

@arielb1

Copy link
Copy Markdown
Contributor

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 25, 2017
…=alexcrichton
Optimize insertion sort
This change slightly changes the main iteration loop so that LLVM can optimize it more efficiently.
Benchmark:
```
name before ns/iter after ns/iter diff ns/iter diff %
slice::sort_unstable_small_ascending 39 (2051 MB/s) 38 (2105 MB/s) -1 -2.56%
slice::sort_unstable_small_big_random 579 (2210 MB/s) 575 (2226 MB/s) -4 -0.69%
slice::sort_unstable_small_descending 80 (1000 MB/s) 70 (1142 MB/s) -10 -12.50%
slice::sort_unstable_small_random 396 (202 MB/s) 386 -10 -2.53%
```
The benchmark is not a fluke. I can see that performance on `small_descending` is consistently better after this change. I'm not 100% sure why this makes things faster, but my guess would be that `v.len()+1` to the compiler looks like it could in theory overflow.
@frewsxcvfrewsxcv mentioned this pull request Mar 25, 2017
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 25, 2017
…=alexcrichton
Optimize insertion sort
This change slightly changes the main iteration loop so that LLVM can optimize it more efficiently.
Benchmark:
```
name before ns/iter after ns/iter diff ns/iter diff %
slice::sort_unstable_small_ascending 39 (2051 MB/s) 38 (2105 MB/s) -1 -2.56%
slice::sort_unstable_small_big_random 579 (2210 MB/s) 575 (2226 MB/s) -4 -0.69%
slice::sort_unstable_small_descending 80 (1000 MB/s) 70 (1142 MB/s) -10 -12.50%
slice::sort_unstable_small_random 396 (202 MB/s) 386 -10 -2.53%
```
The benchmark is not a fluke. I can see that performance on `small_descending` is consistently better after this change. I'm not 100% sure why this makes things faster, but my guess would be that `v.len()+1` to the compiler looks like it could in theory overflow.
@frewsxcvfrewsxcv mentioned this pull request Mar 25, 2017
bors added a commit that referenced this pull request Mar 26, 2017
Rollup of 7 pull requests
- Successful merges: #40642, #40734, #40740, #40771, #40807, #40820, #40821
- Failed merges:
@bors
bors merged commit 2c816f7 into rust-lang:masterMar 26, 2017
@ghost
ghost deleted the optimize-insertion-sort branch March 26, 2017 16:54
Sign up for freeto 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.

6 participants

@rust-highfive@alexcrichton@bors@nagisa@arielb1@aturon