Ensure Firefox frame table subcategory fields are always included

[joshuay03]

Closes #128

I wonder why we set the frame category but not the subcategory here 🤔 Anyway, I've kept it as nil to maintain behaviour while ensuring we conform to the expected Firefox type. See #129 (comment).

[mstange]

I actually found a few more things that should be fixed: When the category is non-null, the subcategory should also be non-null. So it's better to make it an all-zero column rather than an all-null column.
Also, in the central category list, every category should have at least one subcategory, which is the "Other" subcategory at subcategory index zero. I'm adding the missing documentation in firefox-devtools/profiler#5369 .

[mstange]

Oh, one more thing: For cases where the same frame is used by different stack nodes with different categories, those frames need to be split up so that the right stack node category can be computed from the frame category. See firefox-devtools/profiler#5369 (comment)

[joshuay03]

I actually found a few more things that should be fixed: When the category is non-null, the subcategory should also be non-null. So it's better to make it an all-zero column rather than an all-null column.

Noted. I've actually gone ahead and just put the correct subcategory indexes in there for now. I’ll wait for @jhawthorn to see if there’s a reason for not wanting to do that.

Also, in the central category list, every category should have at least one subcategory, which is the "Other" subcategory at subcategory index zero. I'm adding the missing documentation in firefox-devtools/profiler#5369 .

I think we already comply with this here:

vernier/lib/vernier/output/firefox.rb

Line 49 in c4e1980

category.add_subcategory(name: "Other")

[mstange]

Also, in the central category list, every category should have at least one subcategory, which is the "Other" subcategory at subcategory index zero. I'm adding the missing documentation in firefox-devtools/profiler#5369 .

I think we already comply with this here:

vernier/lib/vernier/output/firefox.rb

Line 49 in c4e1980

category.add_subcategory(name: "Other")

Ah, perfect. The profiles which are linked in the readme were probably generated before that change then.

[joshuay03]

Oh, one more thing: For cases where the same frame is used by different stack nodes with different categories, those frames need to be split up so that the right stack node category can be computed from the frame category. See firefox-devtools/profiler#5369 (comment)

Oh interesting, maybe this explains why we don't set the subcategory on the frames. I'll look into reworking this.

[joshuay03]

Oh, one more thing: For cases where the same frame is used by different stack nodes with different categories, those frames need to be split up so that the right stack node category can be computed from the frame category. See firefox-devtools/profiler#5369 (comment)

Oh interesting, maybe this explains why we don't set the subcategory on the frames. I'll look into reworking this.

Okay it seems the frames were intentionally reused in this change, and I don't think that's related to the subcategory omission in any way (that may have just been an oversight/seen as a redundancy). I think I'll merge this as is and tackle the frame separation problem as a separate issue and PR.