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

Improved encoding and decoding speed of Vec<u8> #665

Closed
dtolnay opened this issue Sep 19, 2023 · 3 comments
Closed

Improved encoding and decoding speed of Vec<u8> #665

dtolnay opened this issue Sep 19, 2023 · 3 comments

Comments

@dtolnay
Copy link
Contributor

dtolnay commented Sep 19, 2023

I noticed this comment:

bincode/src/de/impls.rs

Lines 449 to 452 in a22afa3

// TODO: we can't limit `T: 'static` because that would break other things
// but we want to have this optimization
// This will be another contendor for specialization implementation
// if TypeId::of::<u8>() == TypeId::of::<T>() {

You can implement the condition soundly as non_static_type_id::<T>() == TypeId::of::<u8>() where:

use std::any::TypeId;
use std::marker::PhantomData;
use std::mem;

fn non_static_type_id<T: ?Sized>() -> TypeId {
    trait NonStaticAny {
        fn get_type_id(&self) -> TypeId where Self: 'static;
    }

    impl<T: ?Sized> NonStaticAny for PhantomData<T> {
        fn get_type_id(&self) -> TypeId where Self: 'static {
            TypeId::of::<T>()
        }
    }

    let phantom_data = PhantomData::<T>;
    NonStaticAny::get_type_id(unsafe {
        mem::transmute::<&dyn NonStaticAny, &(dyn NonStaticAny + 'static)>(&phantom_data)
    })
}

This is sound because all types with the same TypeId as u8 are u8. In contrast, not all types with the same TypeId as &'static str are &'static str.

@VictorKoenders
Copy link
Contributor

cc #618

Will implement this, thanks

Just to confirm, this is still sound to call with types with a lifetime, e.g. if I make a function like:

fn type_equal<T1, T2>() -> bool {
    non_static_type_id::<T1>() == non_static_type_id::<T2>()
}

// this is fine:
type_equal::<&'static str, u8>();
type_equal::<u8, &'static str>();

// this could result in unsoundness:
// type_equal::<&'a str, &'b str>();

@dtolnay
Copy link
Contributor Author

dtolnay commented Sep 21, 2023

It is sound to get the TypeId for any T using this method. You only need to be concerned about the soundness of unsafe code that downcasts based on the value of the obtained TypeId, as different types can have the same TypeId. So non_static_type_id::<T>() == TypeId::of::<&'static str>() does not mean that you can downcast to &'static str.

@VictorKoenders
Copy link
Contributor

Implemented this in #667. I ended up making a small helper crate for this:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants