Rework structured value casting #396

2020-05-22T12:00:31-05:00

KodrAus commented

2020-05-22 12:00:31 -05:00

(Migrated from github.com)

This is a spike to see if we can improve the way we cast values to concrete types. It does more work when capturing Debug and Display to retain the underlying primitive so that a new macro could reasonably default to creating values through these std traits while retaining numbers.

It does some wacky build script code generation to create a static sorted set of type ids we care about so they can be binary searched quickly.

This is a spike to see if we can improve the way we cast values to concrete types. It does more work when capturing `Debug` and `Display` to retain the underlying primitive so that a new macro could reasonably default to creating values through these std traits while retaining numbers. It does some wacky build script code generation to create a static sorted set of type ids we care about so they can be binary searched quickly.

KodrAus (Migrated from github.com) reviewed

2020-05-22 12:05:08 -05:00

build.rs

					
				@ -2,13 +2,72 @@

				//! atomics and sets `cfg` flags accordingly.

I’m using this so the into_primitive module can tell if it’s in the build script or not. Is there a cfg that exists already for this?

I’m using this so the `into_primitive` module can tell if it’s in the build script or not. Is there a cfg that exists already for this?

src/lib.rs

This seems nice for debug assertions. I’ll look into where stabilisation is at.

I think we just stabilise these const methods on-demand. I can’t see why this one shouldn’t be stabilised, but will look into it.

KodrAus (Migrated from github.com) reviewed

2020-05-23 01:29:05 -05:00

src/lib.rs

See: https://github.com/rust-lang/rust/pull/72488

KodrAus (Migrated from github.com) reviewed

2020-05-23 01:29:25 -05:00

src/lib.rs

See: https://github.com/rust-lang/rust/issues/53485

KodrAus (Migrated from github.com) reviewed

2020-05-26 04:56:45 -05:00

src/kv/value/internal/cast/into_primitive.rs

Specialization is possible because we use impl Trait instead of dyn Trait for inputs. This approach would still be necessary if we switched to dyn Trait. The trade-off would be that with dyn Trait we could generate less code in calling libraries at the cost of still requiring this runtime check.

Specialization is possible because we use `impl Trait` instead of `dyn Trait` for inputs. This approach would still be necessary if we switched to `dyn Trait`. The trade-off would be that with `dyn Trait` we could generate less code in calling libraries at the cost of still requiring this runtime check.

KodrAus (Migrated from github.com) reviewed

2020-05-26 09:15:12 -05:00

src/kv/value/internal/cast/into_primitive.rs

cc @hawkw sorry for the random ping but thought you might find this interesting for tracing 🙂

This is using a static set of type ids sorted at build time and binary searches them for interesting types given a generic T: Debug + ‘static input and treats the input as that concrete type. The runtime cost per argument is small (I measured ~9ns on my machine for a hit and ~5ns for a miss) but is not negligible. It could also use specialisation to remove that runtime cost, which I think might land as min_specialization sometime over the next year. Maybe you could consider it for something like the #[instrument] macro, to keep the nice compatible Debug bound, but also retain numbers and booleans as structured values?

cc @hawkw sorry for the random ping but thought you might find this interesting for `tracing` 🙂 This is using a static set of type ids sorted at build time and binary searches them for interesting types given a generic `T: Debug + ‘static` input and treats the input as that concrete type. The runtime cost per argument is small (I measured ~9ns on my machine for a hit and ~5ns for a miss) but is not negligible. It could also use specialisation to remove that runtime cost, which I think might land as `min_specialization` sometime over the next year. Maybe you could consider it for something like the `#[instrument]` macro, to keep the nice compatible `Debug` bound, but also retain numbers and booleans as structured values?

hawkw (Migrated from github.com) reviewed

2020-05-26 11:30:29 -05:00

src/kv/value/internal/cast/into_primitive.rs

This is fascinating, thanks for pinging me!

KodrAus (Migrated from github.com) reviewed

2020-05-27 22:45:10 -05:00

src/kv/value/internal/cast/into_primitive.rs

So...

If https://github.com/rust-lang/rust/pull/72437 and https://github.com/rust-lang/rust/pull/72488 are both merged then you’d be able to do this at compile time without specialization :)

There’s an example in the const_type_id PR.

So... If https://github.com/rust-lang/rust/pull/72437 and https://github.com/rust-lang/rust/pull/72488 are both merged then you’d be able to do this at compile time without specialization :) There’s an example in the `const_type_id` PR.

KodrAus (Migrated from github.com) reviewed

2020-06-03 08:59:17 -05:00

Cargo.toml

Just testing different strategies for destructuring.

src/kv/value/internal/cast/into_primitive.rs

So strangely enough this approach using const seems slightly slower than the runtime binary search. I haven’t looked into why that is yet.

KodrAus (Migrated from github.com) reviewed

2020-06-03 11:01:08 -05:00

src/kv/value/internal/cast/into_primitive.rs

Just a few missing inlines.

KodrAus (Migrated from github.com) reviewed

2020-06-03 21:25:50 -05:00

src/kv/value/internal/cast/primitive.rs

					
				@ -0,0 +1,195 @@

				/*

@hawkw I’ve got a few different strategies for capturing the value from a generic T here now.

For tracing, if this is something you wanted to do you might end up needing to look more at the specialization one, because the others require a ’static bound in order to get the type id, which from what I can see isn’t compatible with how the #[instrument] macro currently works.

@hawkw I’ve got a few different strategies for capturing the value from a generic `T` here now. For `tracing`, if this is something you wanted to do you might end up needing to look more at the specialization one, because the others require a `’static` bound in order to get the type id, which from what I can see isn’t compatible with how the `#[instrument]` macro currently works.