Skip to content

Mark TypeId::of as inline to avoid monomorphizing unnecessary code #74362

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
alecmocatta opened this issue Jul 15, 2020 · 5 comments · Fixed by #92135
Closed

Mark TypeId::of as inline to avoid monomorphizing unnecessary code #74362

alecmocatta opened this issue Jul 15, 2020 · 5 comments · Fixed by #92135
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@alecmocatta
Copy link
Contributor

alecmocatta commented Jul 15, 2020

TypeId can be used to specialize code by comparing types explicitly (TypeId::of::<u8>() == TypeId::of::<T>(), examples in the wild) or implicitly (<dyn Any>::downcast_mut).

In release mode this works well; in debug mode though the unused impl is unnecessarily monomorphized and codegened, slowing compile time as well as run time.

It isn't a major issue as it only affects debug mode, but I wondered if it was worth marking TypeId::of, <TypeId as PartialEq>::eq, <dyn Any>::is and the various downcast methods as inline(always) to potentially reduce compile times and provide a nice debug mode runtime boost?

use std::any::{Any, TypeId};

struct Container<T>(T);

impl<T: 'static> Container<T> {
    fn foo(&mut self) {
        if let Some(_) = <dyn Any>::downcast_mut::<Container<u8>>(self) {
            // specialized impl
            std::process::abort();
        }
        // general impl
    }
    fn bar(&mut self) {
        if TypeId::of::<u8>() == TypeId::of::<T>() {
            // specialized impl
            std::process::abort();
        }
        // general impl
    }
}

fn main() {
    let mut c = Container(String::new());
    c.foo();
    c.bar();
}

See e.g. abort is present in IR/assembly: Playground

@Alexendoo Alexendoo added C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 22, 2020
@AngelicosPhosphoros
Copy link
Contributor

It would be cool if this calls get inlined in -C opt-level=1. This would improve performance of debug builds of engines like bevy, compiled with weak optimizations.

@AngelicosPhosphoros
Copy link
Contributor

I found that this function always promoted to const on -C opt-level=1
godbolt

Another problem is that ParialEq::eq isn't inlined even if I use #[inline(always)]. Also, manual implementation of PartialEq would remove ability to use it in pattern matching like:
godbolt

#![feature(const_type_id)]

#[inline(always)]
pub const fn is_bool<T: 'static>()->bool{
    use std::any::TypeId;

    const BOOL_ID: TypeId = TypeId::of::<bool>();
    match TypeId::of::<T>(){
        BOOL_ID => true,
        _ => false,
    }
}

pub fn good()->bool{
    is_bool::<bool>()
}

pub fn bad()->bool{
    is_bool::<u8>()
}

@AngelicosPhosphoros
Copy link
Contributor

I think, we should close this issue in favor of #77125

Or in favor of #31844 which allows write such code:

#![feature(specialization)]

use std::marker::PhantomData;

struct IsSameType<T0: 'static, T1: 'static>(PhantomData<T0>, PhantomData<T1>);

trait IsSameTypeTrait{
    const IS_SAME: bool;
}

impl<T0: 'static, T1: 'static> IsSameTypeTrait for IsSameType<T0, T1>{
    default const IS_SAME: bool = false;
}

impl<T> IsSameTypeTrait for IsSameType<T, T>{
    const IS_SAME: bool = true;
}

@AngelicosPhosphoros
Copy link
Contributor

I decided to add #[inline] because we unlikely to get #77125 resolved soon, and #31844 still has soundness problems.

@AngelicosPhosphoros
Copy link
Contributor

AngelicosPhosphoros commented Dec 20, 2021

Adding #[inline] wouldn't have any effect without optimizations unfortunately because there must run InlinerPass which doesn't run on debug.

With New Pass Manager (which I hope would stabilized soon), users may set opt-level=1 for fast debug builds and get profits from this change.

bors added a commit to rust-lang-ci/rust that referenced this issue Dec 24, 2021
…62, r=dtolnay

Add `#[inline]` modifier to `TypeId::of`

It was already inlined but it happened only in 4th InlinerPass on my testcase.
With `#[inline]` modifier it happens on 2nd pass.

Closes rust-lang#74362
@bors bors closed this as completed in 756d163 Dec 24, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants