Skip to content
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

Remove unused profile.json fields? #94

Open
Swatinem opened this issue Feb 14, 2024 · 4 comments
Open

Remove unused profile.json fields? #94

Swatinem opened this issue Feb 14, 2024 · 4 comments

Comments

@Swatinem
Copy link

I enjoyed your FOSDEM 24 talk very much, and I have been exploring the profile.json format a little bit out of curiosity.

There are a bunch of things I noticed which might be specific to the firefox internal profiles which are not relevant for samply profiling any random program.

In particular, there is a ton of fields in the profile.json which I believe do not make any sense outside of firefox, but they are completely filled in with their default values:

  • funcTable.isJS and funcTable.relevantForJs are all false and seem irrelevant
  • frameTable.innerWindowID, frameTable.implementation and frameTable.optimizations are all null and I doubt they are relevant?
  • frameTable.line, frameTable.column, funcTable.fileName, funcTable.lineNumber and funcTable.columnNumber seem to be all null at least all the ones I was looking at. Though filenames are being displayed in the flame graph and opening the source definition works as well, and I have no idea how that even works? :-D
  • frameTable.inlineDepth seems all 0, at least the ones I looked at.

Then there is also frameTable.category, frameTable.subcategory, stackTable.category and stackTable.subcategory. I wonder if the entries in stackTable are duplicated, as the stackTable has a reference to the frameTable which also has the same info, or can the actual values diverge?

In either case, samply only ever outputs a single category (for now, I wish it would be possible to customize this and will open a separate issue for it), so I wonder if this can be compressed a bit better.

@Swatinem
Copy link
Author

Another fun fact here is that the main thread defaults to the name GeckoMain :-D

@vvuk
Copy link
Collaborator

vvuk commented May 21, 2024

These are all written using this SerializableSingleValueColumn((), len) helper, which just writes len nulls. @mstange is this required for the front end? Can we just write an empty array here, and have the front end understand undefined? inlineDepth gets written as len count of 0, so maybe some code to treat undefined as 0 there.

These are taking up a significant amount of space in the json for long multi-process profiles. Even better would be to change the format to make these properties fully optional. For comparison, a ~1 minute capture with 8000 tracks (threads) is around 180MB with all the extra nulls. If I just make SerializableSingleValueColumn just output an empty array, it goes down to 135MB -- which is a very significant 30% size reduction!

@mstange
Copy link
Owner

mstange commented May 21, 2024

In particular, there is a ton of fields in the profile.json which I believe do not make any sense outside of firefox, but they are completely filled in with their default values:

funcTable.isJS and funcTable.relevantForJs are all false and seem irrelevant

These are now seeing some use for profiles with JIT frames. Having two columns is a bit overkill though; they could be combined into a single flags column.

frameTable.innerWindowID, frameTable.implementation and frameTable.optimizations are all null and I doubt they are relevant?

Wait, optimizations is still there? It's no longer needed as of format version 45. So yes we should stop outputting that one immediately.

The removal of implementation is tracked in firefox-devtools/profiler#3713 .

innerWindowID indeed is completely useless for us and could be made optional. I've filed firefox-devtools/profiler#5006 about it.

frameTable.line, frameTable.column, funcTable.fileName, funcTable.lineNumber and funcTable.columnNumber seem to be all null at least all the ones I was looking at. Though filenames are being displayed in the flame graph and opening the source definition works as well, and I have no idea how that even works? :-D

Yes they're null before symbolication and non-null after symbolication, and samply currently only emits unsymbolicated profiles. The filenames are displayed in the flamegraph because the front-end requests symbolication information asynchronously through an extra network request. Once we have a presymbolicate mode, these columns will be non-null in that mode even before the front-end consumes the JSON.

frameTable.inlineDepth seems all 0, at least the ones I looked at.

Same as above - it is often non-zero after symbolication.

Then there is also frameTable.category, frameTable.subcategory, stackTable.category and stackTable.subcategory. I wonder if the entries in stackTable are duplicated, as the stackTable has a reference to the frameTable which also has the same info, or can the actual values diverge?

The values in the stackTable are somewhat duplicated, yes. The reason for this is that the frameTable can have null categories or subcategories - they're always non-null in profiles generated by fxprof-processed-profile, but the format allows null. And then the stackTable always has non-null categories, by inheriting the nearest non-null category from a caller.
In profiles generated by Firefox, all C++ frames have a null category in the frameTable, and the actual categories like Graphics / DOM / JS are supplied by synthetic label frames.

In either case, samply only ever outputs a single category (for now, I wish it would be possible to customize this and will open a separate issue for it), so I wonder if this can be compressed a bit better.

It outputs different categories for User and Kernel on Linux and Windows now. And JIT frames have a different category on all platforms.

Another fun fact here is that the main thread defaults to the name GeckoMain :-D

This was fixed in e74e514 - it had already been fixed at the time this issue was filed, the fix just hadn't made it into a release.

@mstange
Copy link
Owner

mstange commented May 29, 2024

frameTable.innerWindowID, frameTable.implementation and frameTable.optimizations are all null and I doubt they are relevant?

Wait, optimizations is still there? It's no longer needed as of format version 45. So yes we should stop outputting that one immediately.

I've made this change in #232.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants