google / perfetto

Performance instrumentation and tracing for Android, Linux and Chrome (read-only mirror of https://android.googlesource.com/platform/external/perfetto/)
https://www.perfetto.dev
Apache License 2.0
2.86k stars 358 forks source link

tid dropped if >max(uint32) #886

Closed d4l3k closed 6 hours ago

d4l3k commented 1 month ago

The thread IDs from JSON traces are getting dropped if the tid doesn't fit in a uint32. These are displayed finewith chrome://tracing but doesn't work with Perfetto.

I have py-spy chrometrace formatted traces in the format of:

{"args":{"filename":"\u003cfrozen importlib._bootstrap\u003e","line":688},"cat":"py-spy","name":"_load_unlocked","ph":"B","pid":10402,"tid":139627874611776,"ts":9720432}

These are I believe the pthreads thread ID and doesn't fit in a uint32.

https://github.com/google/perfetto/blob/f8a5e8a0fb791ba9b282e3ec334d0e9d4fe9426f/src/trace_processor/importers/json/json_trace_parser_impl.cc#L121

LalitMaganti commented 1 month ago

I'd suggest this is WAI from the Perfetto perspective: please see https://perfetto.dev/docs/faq#why-does-perfetto-not-support-lt-some-obscure-json-format-feature-gt-

d4l3k commented 1 month ago

@LalitMaganti looks like TrackEvent protos are also using int32 for thread ids so doesn't really solve the underlying problem regardless

As a hack it, does look fairly straightforward to just cast it as a string and set it as a threadname instead

https://github.com/google/perfetto/blob/f8a5e8a0fb791ba9b282e3ec334d0e9d4fe9426f/src/trace_processor/importers/json/json_trace_parser_impl.cc#L112-L123

LalitMaganti commented 1 month ago

looks like TrackEvent protos are also using int32 for thread ids so doesn't really solve the underlying problem regardless

Right: the main reason is that supporting int64 thread IDs can be quite a performance concern as it doubles the memory use of all places where tids are stored and when people use us for multi-GB traces, this adds up.

My response was more dealing with "why does this work on chrome tracing but not in Perfetto". You're correct that if you need int64 tid support and there's not a way to emit 32 bit tids instead then yes you'll need some hack.

As a hack it, does look fairly straightforward to just cast it as a string and set it as a threadname instead

That is quite a hack :) can we maybe instead directly cast from int64 -> int32 instead and store the tids in the casted form? We can expose the tids as the name as you suggest then.