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

Long tasks in the report #10705

Closed
connorjclark opened this issue May 5, 2020 · 15 comments
Closed

Long tasks in the report #10705

connorjclark opened this issue May 5, 2020 · 15 comments
Assignees
Labels

Comments

@connorjclark
Copy link
Collaborator

connorjclark commented May 5, 2020

We want to show more information about long tasks, in order to help reduce TTI/TBT.

First: a new "long tasks" diagnostic audit. We've been talking about this. Idea is to show the 5 longest tasks.

Quick way to preview the long tasks–in bootup-time.js:

console.log([...tasks].sort((a, b) => b.duration - a.duration).filter(t => t.duration >= 50));
  • We know the start and end times of each task. Instead of a table, should we explore a timeline visualization, perhaps also adding when TTI/FCP/LCP is?
  • Should we show more than 5? Maybe not all–The verge has ~60 long tasks. 20?
  • What is "Other"? It is different from "unattributable", yes? Should we filter out these two from this audit?

Second: (new idea, please discuss) augment bootup-time with long tasks

We could add subrows for each script, listing the top n long script tasks.

@paulirish
Copy link
Member

Quick way to preview the long tasks–in bootup-time.js:

Nice. I extended that a tad more:

    const longtasks = [...tasks].sort((a, b) => b.duration - a.duration).filter(t => t.duration >= 50 && t.unbounded === false);

    const also = longtasks.map(t => ({
      url: BootupTime.getAttributableURLForTask(t, jsURLs),
      group: t.group.label,
      start: t.startTime,
      self: t.selfTime,
      duration: t.duration
    }));
    console.table(also);

image

Instead of a table, should we explore a timeline visualization,

we can, but let's start with the table. the timeline viz scope can get big really quick so i wanna make sure we ship some basic ground truth and then augment from there.

Should we show more than 5?

good call. up to 20-ish sg.

What is "Other"? It is different from "unattributable", yes? Should we filter out these two from this audit?

different yes. seems like Other happens when most of the time is attributed to events like these. (which seems weird as they are fired at the toplevel/scheduler-level.)

i don't think we should filter either out as Other/Unattributable can still mess up your FID/TBT/TTI

Second: augment bootup-time with long tasks

yeah seems pretty reasonable.

@patrickhulce
Copy link
Collaborator

If we're reporting these to users, you'll want to filter down to toplevel tasks only (which will all be "Other"), use duration not selftime, and then probably aggregate the attributeable URLs of all its subtasks.

tasks.filter(task => !task.parent)
// something here to find all children and combine all URLs

tasks in the context of bootup time are TaskNodes and not what we normally refer to as a task. They're slices of a single toplevel task divided up to correctly attribute URL and execution type.

i don't think we should filter either out as Other/Unattributable can still mess up your FID/TBT/TTI

I agree we should not filter these out. I know it sucks because the site developer can't do anything about it, it's essentially all on chromium developers to lower that cost, but it can be large and important to know where it came from.

augment bootup-time with long tasks

Would definitely like a big heads up before the release of that :) I love the idea, but I know it will break a few things that rely on the structure of bootup-time audit in the HTTPArchive crawl, so it would be awesome if we could time it in between months if possible.

@connorjclark
Copy link
Collaborator Author

connorjclark commented May 5, 2020

If we're reporting these to users, you'll want to filter down to toplevel tasks only (which will all be "Other"), use duration not selftime, and then probably aggregate the attributeable URLs of all its subtasks.

So each long toplevel task could be a row (start time + duration), and there could be subrows (url + task type + duration) for individual subtasks?

Would definitely like a big heads up before the release of that :) I love the idea, but I know it will break a few things that rely on the structure of bootup-time audit in the HTTPArchive crawl, so it would be awesome if we could time it in between months if possible.

Adding subrows shouldn't break programmatic usage.

@patrickhulce
Copy link
Collaborator

So each long toplevel task could be a row (start time + duration), and there could be subrows (url + task type + duration) for individual subtasks?

I was mostly talking about the surfacing of the tasks in bootup time here. I agree, we should feel free to use the full task tree as subrows in the separate audit 👍

Adding subrows shouldn't break programmatic usage.

Fair enough there are definitely ways of doing this in a non-breaking way. I probably misinterpreted what was planned for the table. If it ends up changing any of the status quo: 1 item per script, url is a string, total/scripting/scriptingParseCompile are numbers that represent the aggregate amount of time attributable to that script in each category, then I would greatly appreciate a heads up :)

@brendankenny
Copy link
Member

If this is specifically to help TTI/TBT (and especially if we want to include startTimes), is this going to run into the issue of picking the optimistic or pessimistic lantern graph for TTI?

It's going to be weird to suggest fixing a long task after TTI to improve TTI. Softening the language of what the diagnostic is suggesting (beyond just TTI, long tasks aren't great) could help with that, I guess.

@patrickhulce
Copy link
Collaborator

patrickhulce commented May 6, 2020

It's going to be weird to suggest fixing a long task after TTI to improve TTI

A good point, though I'm not sure this happens that much in practice given our current graph relationships and network quiet settings. It would be very interesting to see though, we have all the data in HA... (EDIT: Doh! no we don't we use their applied throttling 😕)

Nice 🤓 🔫

@connorjclark
Copy link
Collaborator Author

... I probably misinterpreted what was planned ... I would greatly appreciate a heads up :)

FYI, we haven't had any discussions at all about this, it's a new idea I'm seeking feedback on. Sorry I wasn't clear.

@patrickhulce
Copy link
Collaborator

Ah gotcha, from Paul's assignment to @Beytoven I thought it might be imminent. Then consider my feedback a +1 to non-breaking ways of surfacing the subrows in the bootup-time audit! 👍

@Beytoven
Copy link
Contributor

Beytoven commented May 7, 2020

So is there consensus on a preferred approach here? It seems like everyone may be okay with either approach but is anyone leaning more one way or the other? @connorjclark @paulirish @patrickhulce

@connorjclark
Copy link
Collaborator Author

connorjclark commented May 7, 2020

We want to surface the longest 20 toplevel tasks (I think these are marked 'Other' and have no parent) in a table. And for each item, we want subrows of the "longish" tasks within (not sure if this will be a lot of subrows, if so we may want to filter it).

A good start would be everything but the subrow part, and then open a draft PR.

@connorjclark
Copy link
Collaborator Author

First pass for 6.0 let's just do top-level:

image

Subrows aren't ready for the default config.

@connorjclark
Copy link
Collaborator Author

blocked by #10942

@connorjclark connorjclark added 6.2 and removed 6.1 labels Jun 22, 2020
@connorjclark
Copy link
Collaborator Author

image
@patrickhulce does this make any sense? isn't unattributable a catch-all for many non-contiguous tasks?

@patrickhulce
Copy link
Collaborator

Unattributable just means we couldn't get a stack that resolved to any script. This should be attributable once a PR for #8526 lands which I'm free to work on now :)

@paulirish
Copy link
Member

Data is all exposed in the long-tasks audit now. The next big work here is visualizing it to make it more grokkable and actionable. Tracked by #10806

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

No branches or pull requests

6 participants