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

major performance regression #9858

Closed
thestinger opened this issue Oct 14, 2013 · 7 comments
Closed

major performance regression #9858

thestinger opened this issue Oct 14, 2013 · 7 comments
Labels
I-slow Issue: Problems and improvements with respect to performance of generated code.

Comments

@thestinger
Copy link
Contributor

The benchmarks in #8981 had a significant drop in performance on my platform (Linux 3.11.5, glibc 2.18).

Last good commit: 0.8-331-g6db8899
First known bad commit: 0.8-369-g62812f1

@cmr bisected this and identified d858360 as the cause.

cc @alexcrichton

@pnkfelix
Copy link
Member

@cmr bisected this and identified d858360 as the cause.

that ... is bizarre. Looking more deeply now.

@alexcrichton
Copy link
Member

I built before and after that commit on OSX, and I didn't detect a regression either way, although I didn't detect a regression on OSX in #8981 on master either, so I was either measuring incorrectly or this is only a regression on linux.

@pnkfelix
Copy link
Member

I took some samples of builds from four bors-merge points over the last 20 days: sep 24, oct 01, oct 05, and oct 10.

All of the builds were configured with configure --disable-debug --enable-optimize --enable-ccache --enable-clang --prefix=~/opt/rust. My platform is a 2.7 Ghz Intel Core i7 MacBook Pro running OS X 10.8.5.

I do not see a performance regression, at least not on Mac OS X. If anything it seems like things have gotten better for the change proposeed in #8981.

Maybe someone with a Linux machine handy can double-check thestinger's findings.

% for d in sep24 oct01 oct05 oct10 ; do ls -Fd rust-$d-* ; ./rust-$d-*/objdir-opt/x86_64-apple-darwin/stage2/bin/rustc -Aunused-imports --opt-level=3 --test /tmp/b.rs && /tmp/b --bench ; done
rust-sep24-a7d68adbdd75300a4a10b2dcc59e1048f613db48/
warning: no debug symbols in executable (-arch x86_64)

running 7 tests
test test::push ... ignored
test bench::empty ... bench: 5 ns/iter (+/- 0)
test bench::empty_std ... bench: 58 ns/iter (+/- 7)
test bench::push ... bench: 37438 ns/iter (+/- 3493)
test bench::push_preallocated ... bench: 34575 ns/iter (+/- 10314)
test bench::push_preallocated_std ... bench: 34751 ns/iter (+/- 5529)
test bench::push_std ... bench: 43656 ns/iter (+/- 4786)

test result: ok. 0 passed; 0 failed; 1 ignored; 6 measured

rust-oct01-88b0b511beed1599c5bddaf05b9cd0f98bd714ca/
warning: no debug symbols in executable (-arch x86_64)

running 7 tests
test test::push ... ignored
test bench::empty ... bench: 5 ns/iter (+/- 0)
test bench::empty_std ... bench: 59 ns/iter (+/- 6)
test bench::push ... bench: 11623 ns/iter (+/- 2518)
test bench::push_preallocated ... bench: 5882 ns/iter (+/- 744)
test bench::push_preallocated_std ... bench: 37822 ns/iter (+/- 5728)
test bench::push_std ... bench: 42381 ns/iter (+/- 20040)

test result: ok. 0 passed; 0 failed; 1 ignored; 6 measured

rust-oct05-549f70989e3423642bacc41bb6c58ed81d591c03/
warning: no debug symbols in executable (-arch x86_64)

running 7 tests
test test::push ... ignored
test bench::empty ... bench: 5 ns/iter (+/- 0)
test bench::empty_std ... bench: 58 ns/iter (+/- 9)
test bench::push ... bench: 11789 ns/iter (+/- 3440)
test bench::push_preallocated ... bench: 5839 ns/iter (+/- 2173)
test bench::push_preallocated_std ... bench: 37940 ns/iter (+/- 3766)
test bench::push_std ... bench: 42484 ns/iter (+/- 5002)

test result: ok. 0 passed; 0 failed; 1 ignored; 6 measured

rust-oct10-2e1df8e35bb0185dc35f5e7976c0b46608c32404/
warning: no debug symbols in executable (-arch x86_64)

running 7 tests
test test::push ... ignored
test bench::empty ... bench: 5 ns/iter (+/- 0)
test bench::empty_std ... bench: 59 ns/iter (+/- 14)
test bench::push ... bench: 11881 ns/iter (+/- 2696)
test bench::push_preallocated ... bench: 5827 ns/iter (+/- 457)
test bench::push_preallocated_std ... bench: 37639 ns/iter (+/- 4246)
test bench::push_std ... bench: 42216 ns/iter (+/- 7903)

test result: ok. 0 passed; 0 failed; 1 ignored; 6 measured

% 

@pnkfelix
Copy link
Member

(of course I forget to include a build from today in the data above. will add that shortly.) (done)

@emberian
Copy link
Member

I had double-checked his findings. It could be a linux-specific regression.

On Mon, Oct 14, 2013 at 10:06 PM, Felix S Klock II <notifications@github.com

wrote:

I took some samples of builds from four bors-merge points over the last 20
days: sep 24, oct 01, oct 05, and oct 10.

All of the builds were configured with configure --disable-debug
--enable-optimize --enable-ccache --enable-clang --prefix=~/opt/rust

I do not see a performance regression, at least not on Mac OS X. If
anything it seems like things have gotten better for the change proposeed
in #8981 #8981.

Maybe someone with a Linux machine handy can double-check thestinger's
findings.

% for d in sep24 oct01 oct05 oct10 ; do ls -Fd rust-$d-* ; ./rust-$d-*/objdir-opt/x86_64-apple-darwin/stage2/bin/rustc -Aunused-imports --opt-level=3 --test /tmp/b.rs && /tmp/b --bench ; done
rust-sep24-a7d68adbdd75300a4a10b2dcc59e1048f613db48/
warning: no debug symbols in executable (-arch x86_64)

running 7 tests
test test::push ... ignored
test bench::empty ... bench: 5 ns/iter (+/- 0)
test bench::empty_std ... bench: 58 ns/iter (+/- 7)
test bench::push ... bench: 37438 ns/iter (+/- 3493)
test bench::push_preallocated ... bench: 34575 ns/iter (+/- 10314)
test bench::push_preallocated_std ... bench: 34751 ns/iter (+/- 5529)
test bench::push_std ... bench: 43656 ns/iter (+/- 4786)

test result: ok. 0 passed; 0 failed; 1 ignored; 6 measured

rust-oct01-88b0b511beed1599c5bddaf05b9cd0f98bd714ca/
warning: no debug symbols in executable (-arch x86_64)

running 7 tests
test test::push ... ignored
test bench::empty ... bench: 5 ns/iter (+/- 0)
test bench::empty_std ... bench: 59 ns/iter (+/- 6)
test bench::push ... bench: 11623 ns/iter (+/- 2518)
test bench::push_preallocated ... bench: 5882 ns/iter (+/- 744)
test bench::push_preallocated_std ... bench: 37822 ns/iter (+/- 5728)
test bench::push_std ... bench: 42381 ns/iter (+/- 20040)

test result: ok. 0 passed; 0 failed; 1 ignored; 6 measured

rust-oct05-549f70989e3423642bacc41bb6c58ed81d591c03/
warning: no debug symbols in executable (-arch x86_64)

running 7 tests
test test::push ... ignored
test bench::empty ... bench: 5 ns/iter (+/- 0)
test bench::empty_std ... bench: 58 ns/iter (+/- 9)
test bench::push ... bench: 11789 ns/iter (+/- 3440)
test bench::push_preallocated ... bench: 5839 ns/iter (+/- 2173)
test bench::push_preallocated_std ... bench: 37940 ns/iter (+/- 3766)
test bench::push_std ... bench: 42484 ns/iter (+/- 5002)

test result: ok. 0 passed; 0 failed; 1 ignored; 6 measured

rust-oct10-2e1df8e35bb0185dc35f5e7976c0b46608c32404/
warning: no debug symbols in executable (-arch x86_64)

running 7 tests
test test::push ... ignored
test bench::empty ... bench: 5 ns/iter (+/- 0)
test bench::empty_std ... bench: 59 ns/iter (+/- 14)
test bench::push ... bench: 11881 ns/iter (+/- 2696)
test bench::push_preallocated ... bench: 5827 ns/iter (+/- 457)
test bench::push_preallocated_std ... bench: 37639 ns/iter (+/- 4246)
test bench::push_std ... bench: 42216 ns/iter (+/- 7903)

test result: ok. 0 passed; 0 failed; 1 ignored; 6 measured

%


Reply to this email directly or view it on GitHubhttps://github.com//issues/9858#issuecomment-26304375
.

@thestinger
Copy link
Contributor Author

@pnkfelix: the numbers here are basically the reverse of your sept24 -> oct10

@thestinger
Copy link
Contributor Author

This is no longer an issue, since whatever went wrong with jemalloc has been "fixed" by the removal.

flip1995 pushed a commit to flip1995/rust that referenced this issue Nov 21, 2022
`never_loop`: don't emit AlwaysBreaks if it targets a block

ref: rust-lang/rust-clippy#9837 (comment)

The previous fix (rust-lang#9837) was too simple and ignored all break commands inside a labelled block, regardless of whether their destination was a labelled block or a loop. This fix tracks all the labelled blocks in scope to ensure that only breaks targeting loops are considered.

changelog: [`never_loop`]: prevent false negatives from `breaks` nested in labelled blocks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-slow Issue: Problems and improvements with respect to performance of generated code.
Projects
None yet
Development

No branches or pull requests

4 participants