Revert the removal of the "optimizations" property in importers for linux perf and ART

[julienw]

This was done in #4432, but because these importers target a specific version of the gecko profile format, we shouldn't change their structure without upgrading the format too.

Fixes #4547

This is also extremely visible on the traces from our fixtures library, just looking at the category graph.
For example with the file art-trace-regular.trace.gz:
Import with the production code: https://share.firefox.dev/3FTHP9T
Import with this deploy preview: https://share.firefox.dev/3lHuBq7

I considered trying to detect broken uploaded profiles and upgrade them, but not sure this is worth it because the regression is fairly recent. I would love to get your feedback on this @Karn.

[julienw]

Interestingly, we clearly see the fix in this snapshot change (but we can't see this in github because the change is too large)

[codecov]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (ae16685) 88.62% compared to head (5bde5d2) 88.63%.

❗ Current head 5bde5d2 differs from pull request most recent head b761708. Consider uploading reports for the commit b761708 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4549   +/-   ##
=======================================
  Coverage   88.62%   88.63%           
=======================================
  Files         293      293           
  Lines       25989    25982    -7     
  Branches     6998     6996    -2     
=======================================
- Hits        23034    23028    -6     
+ Misses       2749     2748    -1     
  Partials      206      206           

Impacted Files	Coverage Δ
src/profile-logic/import/art-trace.js	91.10% <100.00%> (ø)
src/profile-logic/import/linux-perf.js	96.46% <100.00%> (+0.03%)	⬆️

... and 5 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[Karn]

Thanks for the quick turnaround on this @julienw! The change looks good to me. This might be a trivial question, but what does this mean for:

https://github.com/firefox-devtools/profiler/blob/main/src/profile-logic/gecko-profile-versioning.js#L1435

does there need to be a reversal of this?

[julienw]

Thanks for the quick turnaround on this @julienw! The change looks good to me. This might be a trivial question, but what does this mean for:

https://github.com/firefox-devtools/profiler/blob/main/src/profile-logic/gecko-profile-versioning.js#L1435

does there need to be a reversal of this?

No it's still correct !
This upgrades profiles using the gecko format from version 26 to version 27. The "optimizations" field has been removed. By removing the schema property, we'll simply "jump over" this index.

If you want to know more, this logic is implemented in

profiler/src/profile-logic/process-profile.js

Line 114 in 16e5c34

for (const fieldName in geckoTable.schema) {

[canova]

Thanks for the quick fix!

As we talked briefly, personally I think we should always pin to the latest profile format version instead of pinning to a specific one if we are inside the profiler codebase. We change all the occurances that are inside the profiler codebase when we are changing the format anyways. Pinning to a profiler format version I think makes it more vulnerable to make mistakes like I did in #4432 when we do a find+replace/remove.

Of course pinning to a version is the best option when the importer lives outside of the profiler codebase. I think it would be good to create a guideline for importers so we are more consistent across all of them.
But of course I don't expect to do this here in this PR. It could be nice to file an issue about it at least, what do you think?

[julienw]

Thanks for the review! I filed #4555