core: Consider changing Value to dispatch primitives with an enum rather than trait objects

[davidbarsky]

Feature Request

Crates

• tracing-core

Motivation

As detailed in #922, we have the opportunity to make breaking changes in tracing-core with the release of Tokio 0.3.

Proposal

While I believe that @hawkw and @carllerche have talked more about this, @hawkw shared a sketch of a possible approach.

[KodrAus]

I like the wrapper-struct approach like @hawkw's sketch. I don't want to become the "well, actually in log..." guy, but that's the approach I've taken there, which has let us wrap up both a enum of primitives and trait objects together along with quite a deep compatibility layer that the public API doesn't have to know about.

[hawkw]

I like the wrapper-struct approach like @hawkw's sketch. I don't want to become the "well, actually in log..." guy, but that's the approach I've taken there, which has let us wrap up both a enum of primitives and trait objects together along with quite a deep compatibility layer that the public API doesn't have to know about.

Yup, that was a big part of the inspiration for this change. :)

[KodrAus]

Ah gotcha! 😄

As a future possibility, if you haven’t checked it out already you could also find some inspiration in the way std::fmt erases values by transmuting a &T: Trait into a (&Opaque, fn(&Opaque) pair instead of lots of enum variants. It might make it possible to save some space and boilerplate internally.

[hawkw]

As a future possibility, if you haven’t checked it out already you could also find some inspiration in the way std::fmt erases values by transmuting a &T: Trait into a (&Opaque, fn(&Opaque) pair instead of lots of enum variants. It might make it possible to save some space and boilerplate internally.

Hmm, that's interesting and maybe worth looking into in the future as an optimization! I considered a safe version of this that relies on creating a closure that calls downcast_ref on an Any, much earlier in tracing's development, but that introduces a lot of overhead, which this maybe avoids...might be worth trying at some point.