1626085 - Jank markers should not stack

Status: RESOLVED MOVED

(Core :: Gecko Profiler, defect, P3)

Description

[:Harald Kirschner :digitarald]

Attachment: image.png

https://bit.ly/2QXHNV1 has 3 Jank markers, 2 following each other (mapping to BHR) and a 3rd one that spans the length of the 2.

It isn't expected that Jank markers stack, so something isn't right.

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

I believe these "Jank" markers are created on the frontend from responsiveness data:
https://github.com/firefox-devtools/profiler/blob/master/src/profile-logic/marker-data.js#L30

But this issue could be due to recent responsiveness changes on the Gecko side, e.g. bug 1572337.

Greg, do you know more about this?

Comment 2

[Nazım Can Altınova [:canova]]

Yes, I think that's most likely an issue on the front-end side. We are processing responsiveness values and figuring out those markers there. I worked on the pre-processing of those responsiveness values after the responsiveness -> eventDelay change(bug bug 1572337). But couldn't land it somehow and got bitrotted. It's https://github.com/firefox-devtools/profiler/pull/2282.

I looked at the same profile with this deploy preview and it actually made things worse. There are now 4 jank markers instead of 3. So there is definitely something wrong.

Comment 3

[Julien Wajsberg [:julienw]]

I investigated a bit.

Each marker happens because the "eventDelay" value decreases. But because for these 3 cases it stays above the "jank threshold" maybe we shouldn't generate a jank marker at that moment.

These are the values that generates these 3 jank markers:

marker 1:
sample 2341: eventDelay = 710.
sample 2342: eventDelay = 697

marker 2:
sample 3049: eventDelay = 1403
sample 3050: eventDelay = 705

marker 3:
sample 3077: eventDelay = 732
sample 3078: eventDelay = 0.2

Comment 4

[Julien Wajsberg [:julienw]]

As discussed with Nazim, we will fix this bug as part of https://github.com/firefox-devtools/profiler/pull/2282. This isn't urgent because we actually have the jank information.
So I'm closing this bug here, but share the information in that pull request.
Thanks for filing!