Miscellaneous fixes for tracing #4489
Conversation
|
Thank you for contributing to Miri! |
src/bin/log/tracing_chrome.rs
Outdated
| match self.trace_style { | ||
| TraceStyle::Threaded => { | ||
| if span.fields().field("tracing_separate_thread").is_some() { | ||
| // assign an independent "id" to spans with argument "tracing_separate_thread", | ||
| // so they appear a separate trace line in trace visualization tools, see | ||
| // https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview#heading=h.jh64i9l3vwa1 | ||
| Some(span.id().into_u64()) | ||
| Some(span.id().into_u64() as i64) |
There was a problem hiding this comment.
This might wrap around -- isn't that a problem?
There was a problem hiding this comment.
No, that's by design, I tested and Perfetto accepts 64-bit 2s complement signed integers (i.e. from -2^63 to 2^63-1), so wrapping around is intended
There was a problem hiding this comment.
Then please call cast_signed instead, and ideally add a comment saying that perfetto is fine with negative numbers that for in an i64, but not fine with numbers above i64::MAX. Bare as casts are always sus.
There was a problem hiding this comment.
Then please call cast_signed instead
This is still unresolved
Stypox
force-pushed
the
misc-tracing-fixes
branch
from
a8e42fa to
58d8b16
Compare
|
I improved the markdown comment and also included info about why we cast |
Perfetto gives an error if an id does not fit in an 64-bit signed integer in 2's complement.
Stypox
force-pushed
the
misc-tracing-fixes
branch
from
58d8b16 to
0a8a3a1
Compare
This PR contains a few misc fixes for the tracing code that was recently added:
u64toi64so that it can always be loaded in Perfetto (otherwise it would sometimes be>2^63-1and Perfetto wouldn't be able to parse it properly, e.g. test these: trace-ok.json, trace-error.json).$in the macro (sorry about this, I probably erased it by accident right before committing)MIRI_TRACING=1is provided without--features=tracing, ref: Add tracing_chrome under "tracing" feature #4406 (comment)@rustbot ready