-
Notifications
You must be signed in to change notification settings - Fork 369
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
set timestamp format only affects timestamp and not duration formatting #622
Comments
Please see https://perfetto.dev/docs/faq#why-does-perfetto-not-support-lt-some-obscure-json-format-feature-gt- for our stance on supporting the Chrome JSON format. Also keep in mind the JSON format was designed for Chrome: anyone other usecase was never officially supported, it just happened to work. |
Patches for supporting this are welcome but it's not something we'd get to ourselves. |
Perfetto really needs to define a json format of it's own if this is unsupported. Solely relying on protobuf use is limiting. Also the "raw" timestamp when displayed raw isn't what's set from the json It's converting the value to ns. These are meant to be byte values. 131072000 <- the value should be 131072. It's adding 3 0's at the end. |
This is not specific to do with the unsupported Catapult json format, and is about not being able to control the formatting of duration in the app. It's unclear why this command overrides the timestamp and not the duration. Also the ability to hover over and see the unabbreviated name and duration are still needed. |
We don't really have any intention of doing this in the short or medium term.
This matches the expected behaviour of Chrome traces which emits events in microseconds wheras everything in the Perfetto world is always based on nanoseconds: this requires multiplying by 1000 which is what you're seeing. As mentioned in the FAQ, if this doesn't happen to work for other usecases of chrome://tracing, then again patches welcome but not something we will get to ourselves. |
This ia problem in general, not just with Catapult import. I want to see the timestamp duration unformatted. One of your devs stated that there should be more flexibility on this, but I haven't seen this change in 3 years. I need to see my duration as a single value (for timing and for memory tracking). If it has to have 3 000 on the end, that is fine, but at least the Desired case below is way easier to parse that the formatted version that Perfetto currently displays. This should also be show in a hover tip. Displayed:1m 4s 8ms |
We did have support for this at some point via supporting displayTimeUnit (see #141 (comment), ee80bcb) but it caused too many problems so we got rid of it. I don't think we'll be brining it back given how many problems it caused.
The problem here is coming between a disconnect between how you are wanting to use Perfetto and what Perfetto is fundamentally. Perfetto is not a charting system which can chart arbitrary data. It's a timeline viewer used for visualizing traces generated using Perfetto tooling. This is also the same for Catapult as well. If Perfetto happens to work for cases outside our strict aims, that's great we're very happy that it can be useful. However, we have no intention of supporting cases where implementing a feature would go against the design direction or the aims of the project. For example, here, it does not make any sense for us to support a feature where timestamp is not actually a timestamp but something totally different. This also ties with our approach to JSON vs protobuf: again our goal is not to be a charting tool for everyone but a way to visualize Perfetto traces. Perfetto traces are protobuf based so if you want the full feature set of Perfetto, then you need to use protobuf. We happen to support a JSON format for Chrome legacy usecases but purely for those usecases. For any other non-Chrome uses of Catapult, we will not go out of our way to support them. Hope this clears up our approach to bugs like this in general. |
There isn't a disconnect here. I've read the passage about how little effort Perfetto wants to support that Json format. There is no Python wrapper to write the format, but I'm well versed in C++ and can go direct to the API. But I was supplying the visualizer above to AMD's VMA library that has to run on multiple platforms, so Python was convenient. I use Perfetto for timing, but also happened to use it for a memory trace. We actually were using the displayTimeUnit, so if that was gotten rid of, then it might have broken our traces. In both cases, I want to see the unformatted timing. Just the timing that we specify, or a less formatted version (as per above). And also see the name/duration in a hover tip for super small values that are unreadable without having to change zoom. These would also happen to help the memory tracking case. |
Here's an example from our timings, not a memory trace. We record all timings in ms, so showing them in seconds isn't ideal. And showing them in the current split hrs/s/ms/micros/ns is even less readable. The timestamp here I can somewhat read even though I'd prefer to not see it displayed in seconds And I've already set "set timestamp format" to seconds. But there's a disconnect between the timestamp and duration displayed. I'm actually fine if you want to convert our timings to ns. Just need more options for what is displayed, and should strip trailing 0's. That's why I can never use the "raw" view for my memory report. This is my work on the VMA memory display over here. But it hints at what I've tried to do. |
@stevegolton is there something better we can do here? |
HI @alecazam, IIUC you have two requests of the UI (let me know if I've missed anything).
1 is trivial to implement, but duration formatting is a controversial topic. It might be difficult to apply a single duration format to all parts of the UI. Let us discuss internally how this is going to work. 2 is an entirely reasonable request, but is a larger chunk of work. We are currently doing some work on tracks to reduce the total number of different track implementations, at which point it should be a lot easier to introduce features like this to all tracks in one go. Thus I'll be a while before we get around to this, but we are always happy to accept patches. |
|
I've added a patch to address the duration issue, you can see the results here. This might not be it's final form, though early feedback is welcome. I also added the ability to change the time format from a dropdown menu on the timestamp/duration itself. |
That duration formatting looks great, and consistent. Now the timeline, timestamp, and duration all match the same formatting. This means I'm not having to convert between representations, and I can read both time and memory profiles much easier. One recommendation would be to have a "milliseconds" setting, since for games, out timings are all in milliseconds. When you have 360Hz displays, seconds really is too big of a unit, and milliseconds is even getting to be too large (only 3ms per frame). But I can get by and will often use seconds too. |
Great! Just waiting to get some feedback on it before landing. I'll probably tackle the milliseconds in another CL. |
Just landed, should be available on the autopush channel soon. |
The duration format follows the global duration timestamp settings. Screenshot: https://screenshot.googleplex.com/3nX3WZ3kZwrZEpM.png Bug: #622 Change-Id: I4e29bffef3f18e82cf003efbbbfbf6ff408b7f8c
I need both 'raw' to apply to the timestamp and duration. I'm using Perfetto to display memory blocks that are not time-based. And I really don't need 3 chunks of formatted values for each timestamp. I just want the values even when I do use Perfetto for timings.
The text was updated successfully, but these errors were encountered: