Refactored code to access TLS only in case of panic

[nikhilshagri]

Fixes #34787
r? @alexcrichton

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[alexcrichton]

Thanks @cynicaldevil! Two things I'm curious about:

• Could the debug_assert! be moved to the happen just before the return? It shouldn't be perf sensitive and should be a valid assertion for both paths.
• Could you take a look at some benchmark numbers and see how much this affects the perf of a no-panic catch_unwind?

[nikhilshagri]

Sure! no problem.
On 15 Jul 2016 10:12 pm, "Alex Crichton" notifications@github.com wrote:

Thanks @cynicaldevil https://github.com/cynicaldevil! Two things I'm
curious about:

• Could the debug_assert! be moved to the happen just before the return?
  It shouldn't be perf sensitive and should be a valid assertion for both
  paths.
• Could you take a look at some benchmark numbers and see how much
  this affects the perf of a no-panic catch_unwind?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#34836 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AKvBjqgVYZk8wg6LzkMSG5Ous23ln6iUks5qV7hYgaJpZM4JNgaz
.

[nikhilshagri]

do I need to pass additional flags while compiling or is the - -bench
flag enough?
On 15 Jul 2016 10:34 pm, "Nikhil Shagrithaya" nikhilshagri@gmail.com
wrote:

Sure! no problem.
On 15 Jul 2016 10:12 pm, "Alex Crichton" notifications@github.com wrote:

Thanks @cynicaldevil https://github.com/cynicaldevil! Two things I'm
curious about:

• Could the debug_assert! be moved to the happen just before the
  return? It shouldn't be perf sensitive and should be a valid
  assertion for both paths.
• Could you take a look at some benchmark numbers and see how much
  this affects the perf of a no-panic catch_unwind?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#34836 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AKvBjqgVYZk8wg6LzkMSG5Ous23ln6iUks5qV7hYgaJpZM4JNgaz
.

[alexcrichton]

Yeah you'll want something like:

#![feature(test)]
extern crate test;

#[bench]
fn foo(bh: &mut test::Bencher) {
    bh.iter(|| /* thing to benchmark */);
}

$ rustc --test foo.rs -O
$ ./foo --bench

[nikhilshagri]

I put the assert statement before the return now, and also ran the benchmarks:

#![feature(test)]
#![allow(warnings)]
extern crate test;

use std::panic::catch_unwind;

#[bench]
fn test_panic(bh: &mut test::Bencher) {
    bh.iter(|| {
        catch_unwind(|| {
          println!("Hello World!");
        });
    });
}

my-toolchain

bench:       5,995 ns/iter (+/- 1,333)

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

nightly

bench:       6,030 ns/iter (+/- 1,330)

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

Tbh, there isn't much of a difference, I ran these a few times and the time difference was always in the range of 100ns. Sometimes, the nightly was faster than my custom build. Tested on Ubuntu.

[alexcrichton]

Oh actually could this be just a micro-benchmark for catch_unwind performance? For example, just:

catch_unwind(|| {})

It's true yeah that catch_unwind should always be dwarfed by I/O

[nikhilshagri]

@alexcrichton Here it is:

#![feature(test)]
#![allow(warnings)]
extern crate test;

use std::panic::catch_unwind;

#[bench]
fn test_panic(bh: &mut test::Bencher) {
    bh.iter(|| {
        catch_unwind(|| {});
    });
}

my-toolchain

running 1 test
test test_panic ... bench:          12 ns/iter (+/- 1)

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

nightly

running 1 test
test test_panic ... bench:          13 ns/iter (+/- 8)

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

[alexcrichton]

@bors: r+ b28e9dc

Ok, so not a huge win perhaps on Linux, (verified locally as well), but I suspect on WIndows this will make it much faster!

[alexcrichton]

Oh dear, shouldn't have hit the merge button...

I guess let's see if it passes!

[eddyb]

@alexcrichton I don't think it did: https://buildbot.rust-lang.org/builders/auto-win-gnu-32-opt-rustbuild/builds/1774

[TimNN]

https://buildbot.rust-lang.org/builders/auto-win-gnu-32-opt-rustbuild/builds/1776 as well

[alexcrichton]

Yeah sorry about that! I've merged the revert, @cynicaldevil would you mind reopening and I'll r+ correctly this time around?