Reduced performance when using question mark operator instead of try!

This was reported on the users forum , and I don't want it to get lost. Basically, replacing try! by ? resulted in ~20% performance loss in benchmarks:

ratel-rust/ratel-core#48 (comment)

I've reproduced, but not further investigated, these findings. Is that expected right now? It's not a good argument for adopting the question mark :)

I didn't read the implementation until now, I though they expanded to the exact same thing as try!()

Might just need to stick a couple of #[inline]s on Carrier methods.

I'm curious why this issue doesn't even get a label like I-Slow.

@sfackler

@sfackler I tried this (#[inline] on the trait method definitions and the impl methods), but didn't seem to get any different bench numbers.

@arielb1

@arielb1 I've implemented a test version of the "variant copy prop" by hacking into CopyPropagation. However @eddyb has unfortunately 😉 said that it's much better to make a more general version of that that can propagate copies of aggregates (somehow).

improve build_index performance by 5-10%

Rollup merge of rust-lang#42275 - scottmcm:try-trait, r=nikomatsakis

I think this issue is fixed.

Generated asm is same for try! and ?.

https://play.rust-lang.org/?gist=625d88df305ace951a088e1cee2ec13a

Edit: Typo

Even if this is fixed: Is there a unit test which prevents regression?

We could have a codegen test for this.

Someone tag this as E-needstest please...

@mati865

@mati865

I know that. It has a bug that I couldn't quite pin out.

Still not fixed (tested nightly on playpen)

https://play.rust-lang.org/?version=nightly&mode=release&edition=2015&gist=b3858efebcadbffcbe2155e3f2750d07

This seems to be fixed (in stable). Tried the play link above.

; Function Attrs: noinline norecurse nounwind readnone uwtable
define { i64, i64 } @try_op(i64, i64) unnamed_addr #2 {
  %3 = tail call { i64, i64 } @try_macro(i64 %0, i64 %1) #2
  ret { i64, i64 } %3
}

try_op:
	jmp	try_macro

try_macro:
	xorl	%eax, %eax
	testq	%rdi, %rdi
	setne	%al
	movq	%rsi, %rdx
	retq

You can still provoke it to make a difference and introduce conditionals or more copies for the ? operator than for the try!() macro if you change the payload types for the Result, for example using (i32, i32) or String.

type T = (i32, i32);
type E = T;
type R = Result<T, E>;

#[no_mangle]
pub fn try_op(a: R) -> R {
    Ok(a?)
}

#[no_mangle]
pub fn try_macro(a: R) -> R {
    Ok(try!(a))
}

Looks like it regressed: https://godbolt.org/z/awKD_U

We are fast again (at least for the examples given above) since Rust 1.52 which contains LLVM 12 upgrade!

Once somebody adds test (or confirms that similar test already exists) this issue could be closed.

There's this codegen test, but I'm not sure if it's enough to check for this.

According to this blog post, Rust has to this date (Rust 1.62.1) performance problems with the question mark operator, resulting in a 4% performance loss.

but I'm not sure if it's enough to check for this.

Definitely not -- you'll notice it's both -Zunsound-mir-opts and FIXME: broken.

But there's good news! LLVM 15 merged a few days ago, bringing the fix mentioned in #85133 (comment).

With that, both of these are nops now:

#![feature(try_blocks)]

pub fn result_nop_match(x: Result<i32, u32>) -> Result<i32, u32> {
    match x {
        Ok(x) => Ok(x),
        Err(x) => Err(x),
    }
}

pub fn result_nop_traits(x: Result<i32, u32>) -> Result<i32, u32> {
    try {
        x?
    }
}

https://rust.godbolt.org/z/71dYnrMf6

example::result_nop_match:
        mov     rax, rdi
        ret

example::result_nop_traits:
        mov     rax, rdi
        ret

For comparison, on 1.63 even the match version needs a bunch of code: https://rust.godbolt.org/z/oarec37sG

example::result_nop_match:
        xor     ecx, ecx
        test    edi, edi
        setne   cl
        movabs  rax, -4294967296
        and     rax, rdi
        or      rax, rcx
        ret

EDIT: I opened #100693 to have a codegen test here.

@scottmcm

Triage: #100693 added the test for this case (thank you @scottmcm!), closing as fixed.

Navigation Menu

Repository navigation

Reduced performance when using question mark operator instead of `try!` #37939

5 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Reduced performance when using question mark operator instead of try! #37939

Description

Activity

arthurprs commented on Nov 24, 2016

sfackler commented on Nov 24, 2016

StefanoD commented on Nov 26, 2016

birkenfeld commented on Dec 3, 2016

bluss commented on Dec 11, 2016

kdy1 commented on Sep 28, 2017

StefanoD commented on Sep 28, 2017

kennytm commented on Sep 28, 2017

5 remaining items

arielb1 commented on Feb 1, 2018

kornelski commented on Apr 24, 2018

mehcode commented on Aug 31, 2018

bluss commented on Dec 1, 2018

mati865 commented on Apr 3, 2019

mati865 commented on Aug 17, 2022

Kobzol commented on Aug 17, 2022

StefanoD commented on Aug 17, 2022

scottmcm commented on Aug 17, 2022

JohnTitor commented on Dec 21, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Reduced performance when using question mark operator instead of `try!` #37939