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 Animation Frame (LoAF) explainer #100

Merged
merged 17 commits into from
Mar 29, 2023
Merged

Long Animation Frame (LoAF) explainer #100

merged 17 commits into from
Mar 29, 2023

Conversation

noamr
Copy link
Contributor

@noamr noamr commented Feb 7, 2023

Add explainer for long-task revamp

See w3c/event-timing#122 and #89

@noamr noamr closed this Feb 7, 2023
@noamr noamr reopened this Feb 7, 2023
Copy link
Contributor

@mmocny mmocny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exciting!

loaf-explainer.md Outdated Show resolved Hide resolved
loaf-explainer.md Outdated Show resolved Hide resolved

- Time spent in forced layout/style calculations - e.g. calling `getBoundingClientRect`, doing more processing, and then rendering (also known as "layout thrashing" or "forced reflow").
- How many rendering opportuinities were missed - counting points in time where the browser was ready to receive a new paint but the main thread was busy with this LoAF.
- Is the frame blocking animation/input-feedback *in practice*. Note that a frame that blocks actual UI events would also be accessible via [event timing](https://w3c.github.io/event-timing/).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Animations are very complicated, to the point that we should consider not including them in scope.

Many animations are declarative and not driven by main thread updates. Many animations start/stop into sequences of frames, and it tends to be more useful to consider the overall effect of long sequences of frames.

Once we have LoAF reporting, I think it will make it easier to revisit this problem.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(though, when it comes to attribution, perhaps some signal about "had animation" or "had event" would be useful)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, that's what I was thinking

entryType: "long-animation-frame",

// https://html.spec.whatwg.org/#event-loop-processing-model, see |taskStartTime|
startTime: taskStartTime,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this be the first task, first long task, or first task with some invalidation/rendering change?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...the pseudocode at the bottom answered this question... it would be the last task before rendering.

The theory is that rendering always follows a long task, and that a bunch of small tasks cannot collectively add up to a long frame.

I think these can be tested over time, but I think its a reasonable hypothesis to start with.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I changed this - it's wrong. It should be the first task after the last "update the rendering" that had a rendering opportunity.

loaf-explainer.md Outdated Show resolved Hide resolved

// The number of times there was a render opportunity but the main thread was busy
// processing this frame
discardedRenderOpportunities,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative: expose the previous paintTiming (in case the previous animation frame was not long).

Or, maybe duration is entirely sufficient.

discardedRenderOpportunities,

// Whether this long frame was blocking input/animation in practice
blocking: 'ui-event' | 'animation' | 'none',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can have ui-event and animation at the same time...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think in that case we'd say we're blocking a UI event.

const startTime = performance.now();
const task = eventQueue.pop();
if (task)
task.run();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you mentioned-- there is some surprising detail here, in that task.run() can do a lot more than perhaps expected, including run other, nested "event loops", such as dispatching UI events.

Also includes microtasks.

loaf-explainer.md Outdated Show resolved Hide resolved

### Some notes about processing

1. relying on "discarded rendering opportunities" as the qualifier for sluggishness alongside (or
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part feels more risky and out of place than the bulk on the proposal. I would consider decoupling animations to the future.

Regarding measurement and tab visibility -- I would consider factoring something like rendering priority, instead? There are time periods when rendering is high, normal, or low/no priority. Perhaps we treat these differently?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rendering priority is an unspecified implementation-specific thing.
I think at least to begin with I'd keep it well lit and only measure active visible documents that remain that way throughout the frame.

@noamr
Copy link
Contributor Author

noamr commented Mar 29, 2023

Maybe we can merge this into main some time?

Copy link
Contributor

@yoavweiss yoavweiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few nits but otherwise LGTM

loaf-explainer.md Outdated Show resolved Hide resolved
loaf-explainer.md Outdated Show resolved Hide resolved
loaf-explainer.md Outdated Show resolved Hide resolved
@noamr noamr merged commit aa77d83 into main Mar 29, 2023
@noamr noamr deleted the loaf-explainer branch March 29, 2023 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants