1578354 - DevTools Performance Tool Flame Chart unable to display call stack

Status: VERIFIED FIXED

(DevTools :: Performance Tools (Profiler/Timeline), defect, P1)

Description

[Claudio Cocciarelli]

Attachment: Schermata 2019-09-03 alle 10.24.04.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:71.0) Gecko/20100101 Firefox/71.0

Steps to reproduce:

• Open the Performance tool in DevTools and begin to record
• Do any activity on the web (ex. search "firefox" on google.com then switch to Video)
• Stop the recording and select the Flame chart JS tab

The bug only affects recent builds such as Nightly 71 and Firefox Developer Edition 70, Firefox stable 68.0.2 is not affected.

Actual results:

No Call stack is shown (see attachment with dark background, Firefox Nightly 71)

Expected results:

A correct Flame Chart is shown (see attachment with light background, Firefox 68.0.2)

Comment 1

[Claudio Cocciarelli]

Attachment: Schermata 2019-09-03 alle 10.22.34.png

Comment 2

[Julien Wajsberg [:julienw]]

Looks like we have no stack at all in v69. This is super bad because this makes the tool completely useless, and v69 is released today.

Comment 3

[Pascal Chevrel:pascalc]

Bogdan, could your team help us find the regression range? Thanks!

Comment 4

[Bogdan Maris, Desktop Test Engineering]

(In reply to Pascal Chevrel:pascalc from comment #3)

Bogdan, could your team help us find the regression range? Thanks!

Sure thing! Daniel, can you please take this for a spin?

Comment 5

[Daniel Cicas [:dcicas], Release QA]

Hello,

• First bad: 2019-07-03
• Last good: 2019-07-02
• Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=5607d2f3ef7db1adb3f42102c905d47ed8030c49&tochange=502b93be5c7c9dea5f72c71e324e81820f9e5573
• Potentially regressed by:
  Bug 1562892 - WR capture native fonts as raw r=lsalzman

we save the native fonts by their full path now. On macOS, there is no
such thing as a full filesystem path for a CGFont (or at least we don't track it),
so loading a capture falls back to the old logic of using the dummy font.

I hope this helps. If I can help with anything else NI me.

Comment 6

[Julien Wajsberg [:julienw]]

Thanks for the regression window! I'm pretty sure this has rather been regressed by Bug 1557789, some format have been changed and probably we have to change it in this tool as well.

Comment 7

[Julien Wajsberg [:julienw]]

I found the issue and will write a minimal patch tomorrow.
In bug 1557789 we now emit a category for JIT frames [1], but the code in [2] will filter out these frames.

[1] https://hg.mozilla.org/integration/autoland/rev/1f407c83db16381332d82bcfa992eea1022284ac#l5.71
[2] https://searchfox.org/mozilla-central/rev/9bb55ae4d808fc48afcf93f99da6a685265b86c6/devtools/client/performance/modules/logic/frame-utils.js#208-212

Comment 8

[Julien Wajsberg [:julienw]]

Claudio, as a work around, you can set the option "Show Gecko Platform Data"; you'll have more information than you're used to, but at least you'll be able to access the stacks.

Comment 9

[Julien Wajsberg [:julienw]]

Attachment: Bug 1578354 - Change the condition that filters out cpp frames when computing the call tree

Bug 1557789 added categories to all JS frames, so as a result JS frames
where all filtered out because of the condition changed in this patch.
We now check for the JS category as well.

Comment 10

[Julien Wajsberg [:julienw]]

Attachment: Bug 1578354 - Remove the obsolete condition so that JS frames are now properly filtered in in the call tree

Bug 1557789 added categories to all JS frames, so as a result JS frames
where all filtered out because of the condition removed in this patch.
This condition is basically a premature optimization, removing it
shouldn't bring any behavior difference.

Comment 11

[Julien Wajsberg [:julienw]]

Attachment: Bug 1578354 - Remove the obsolete condition so that JS frames are now properly filtered in in the call tree

Bug 1557789 added categories to all JS frames, so as a result JS frames
where all filtered out because of the condition removed in this patch.
This condition is basically a premature optimization, removing it
shouldn't bring any behavior difference.

Comment 12

[Julien Wajsberg [:julienw]]

Comment on attachment 9090658 [details]
Bug 1578354 - Remove the obsolete condition so that JS frames are now properly filtered in in the call tree

Beta/Release Uplift Approval Request

• User impact if declined: By default, the stacks are not visible in the call tree and the flame graph, making the tool nearly useless. The user has to enable "show Gecko Platform Data" which shows more data than is needed for an average web developer.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a small and self-contained patch. It also has a lot of tests attached to it (sadly not enough to find this regression but well...).
  This patch is only for the release branch as another more complex patch will be done for nightly and beta.
• String changes made/needed: none

Comment 13

[Julien Wajsberg [:julienw]]

Comment on attachment 9090391 [details]
Bug 1578354 - Remove the obsolete condition so that JS frames are now properly filtered in in the call tree

The more complex change will go to a separate bug.

Comment 14

[Julien Wajsberg [:julienw]]

Here is another try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c7e4adee321b45aa4ae7d0ae2c21d5719c5dfc35

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6f265c2b6735
Remove the obsolete condition so that JS frames are now properly filtered in in the call tree r=gregtatum

Comment 16

[Julien Wajsberg [:julienw]]

Comment on attachment 9090658 [details]
Bug 1578354 - Remove the obsolete condition so that JS frames are now properly filtered in in the call tree

Beta/Release Uplift Approval Request

• User impact if declined: By default, the stacks are not visible in the call tree and the flame graph, making the tool nearly useless. The user has to enable "show Gecko Platform Data" which shows more data than is needed for an average web developer.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a small and self-contained patch. It also has a lot of tests attached to it (sadly not enough to find this regression but well...).
  This patch is only for the release branch as another more complex patch will be done for nightly and beta.
• String changes made/needed: none

Comment 17

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/6f265c2b6735

Comment 18

[Julien Wajsberg [:julienw]]

Hey Claudio,
The fix should be in the latest nightly.
Can you please double check that this works for you in that version?
Thanks in advance!

Comment 19

[Claudio Cocciarelli]

Hi Julien,
just downloaded the latest nightly and yes, in my use case it now works as it did before. Thank you guys for the quick fix!

Comment 20

[Julien Wajsberg [:julienw]]

Thanks for the check!
Actually while investigating, I found that we were probably hiding some javascript frames, but not all... for several years. This was fixed as part of this fix too.

Comment 21

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9090658 [details]
Bug 1578354 - Remove the obsolete condition so that JS frames are now properly filtered in in the call tree

Fix for regression, includes test fixes, verified in nightly.
Ok for uplift for beta 5.

Comment 22

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/releases/mozilla-beta/rev/eb147c90d7db

Comment 23

[Daniel Cicas [:dcicas], Release QA]

Hello,

I can confirm this issue is fixed on Fx 70.0b5 I verified on Win 10 x64, mac OS 10.13 and Ubuntu 18.04.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9090658 [details]
Bug 1578354 - Remove the obsolete condition so that JS frames are now properly filtered in in the call tree

Fixes a devtools regression causing stacks to not show. Approved for 69.0.1.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/50fa235ffdd0

Comment 26

[Ryan VanderMeulen [:RyanVM]]

Added to the 69.0.1 relnotes:

Fixed missing stacks in the Developer Tools Performance section

Comment 27

[Daniel Cicas [:dcicas], Release QA]

Hello,

I can confirm that this issue does not occur on Fx 69.0.1 RC on Win 10 x64, Ubuntu 18.04 and mac OS 10.13.