Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

chinmaygarde
Copy link
Member

Under normal circumstances, all Impeller pipelines are ready well before the first frame begins processing. This is true even in cases where there is no existing pipeline cache. When there is a pipeline cache, startup is even faster. A device specific pipeline cache is saved after first-start.

However, low-end devices like the Mokey are... really low-end. Pipeline setup (via the ContentContext) may be the bottleneck. In such cases, the observation is that not all pipelines need to be ready for frame rendering to begin. But the pipelines that are needed for the first frame must be prioritized before the rest.

This patch adds a job queue where job priorities can be controlled by augmenting blocking accesses to the pipeline futures by making them bump up the priority of the pending job in the job queue.

On most devices, this will do nothing as all pipelines will be ready. On really low-end devices, this will time to first frame on first launch.

Additionally, traces have been augmented to make it more apparent which pipeline is causing the startup stall. This can aid in reordering first-frame critical pipelines on low-end devices. If you click on the PipelineVK::Create call, the name of the pipeline being created will be added to the metadata.

This change in itself reduces the raster time of the first frame on the mokey from ~555 ms to ~130 ms. The additional tracing also shows that the entire remaining stall is caused by the glyph atlas pipeline. Moving that for eager construction should get back nullify all regressions due to pipeline setup even on the Mokey.

Verified this is does nothing on all other devices.

Before

Notice the first frame raster time.

Screenshot 2024-08-05 at 3 55 48 PM

mokey_before.json.zip

After

The traces end after the first frame in the after because of --trace-startup. But the pipeline continue being built as before.

Screenshot 2024-08-05 at 3 56 42 PM

mokey_after.json.zip

Under normal circumstances, all Impeller pipelines are ready well before
the first frame begins processing. This is true even in cases where
there is no existing pipeline cache. When there is a pipeline cache,
startup is even faster. A device specific pipeline cache is saved after
first-start.

However, low-end devices like the Mokey are... really low-end. Pipeline
setup (via the ContentContext) may be the bottleneck. In such cases, the
observation is that not all pipelines need to be ready for frame
rendering to begin. But the pipelines that are needed for the first
frame must be prioritized before the rest.

This patch adds a job queue where job priorities can be controlled by
augmenting blocking accesses to the pipeline futures by making them bump
up the priority of the pending job in the job queue.

On most devices, this will do nothing as all pipelines will be ready. On
really low-end devices, this will time to first frame on first launch.

Additionally, traces have been augmented to make it more apparent which
pipeline is causing the startup stall. This can aid in reordering
first-frame critical pipelines on low-end devices.
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

///
/// @param[in] id The job identifier.
///
void PrioritizeJob(UniqueID id);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be used somewhere? My understanding is that every pipeline would be scheduled to be created, then there is a mechanism to boost the priority when the pipeline is requested?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes! I was going to add this the spot where the glyph atlas pipeline was being setup. Still WIP.

Copy link
Member Author

Choose a reason for hiding this comment

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

Every pipeline will be scheduled eventually in the order in which the jobs were added. Jobs can skip to the front of the queue as necessary (this call). Jobs can be performed eagerly on anything too to avoid a context switch.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to re-prioritize the glyph atlas shader? Can we simply move it to the front of the queue in ContentContext?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think thats what I mean? Just call PrioritizeJob on it immediately after?

Copy link
Member Author

Choose a reason for hiding this comment

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

Roger, will re-arrange the pipeline and remove PrioritizeJob. Keep the jobqueue still or pass?

Copy link
Member Author

Choose a reason for hiding this comment

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

The stall is completely gone (at least in this app) after the rearrange.
Screenshot 2024-08-06 at 11 52 55 AM

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we'll need the job queue

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I think so too. Hopefully. Just checking the gallery and if I can suppress the regression on the mokey, that should be it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Closing in favor of #54373

chinmaygarde added a commit to chinmaygarde/flutter_engine that referenced this pull request Aug 6, 2024
Fixes the regression to the first-frame time on the Mokey.

Also add a trace that should allow for easier identification of problematic pipelines.

A more involved job-queue that allows construction jobs to skip the queue was written in flutter#54355. But the Mokey case is not important enough for the additional complexity. See the context in the linked patch.
@chinmaygarde chinmaygarde deleted the jumpstart branch August 6, 2024 20:25
auto-submit bot pushed a commit that referenced this pull request Aug 6, 2024
Fixes the regression to the first-frame time on the Mokey.

Also add a trace that should allow for easier identification of problematic pipelines.

A more involved job-queue that allows construction jobs to skip the queue was written in #54355. But the Mokey case is not important enough for the additional complexity. See the context in the linked patch.

<img width="2137" alt="Screenshot 2024-08-06 at 1 20 26�PM" src="https://github.com/user-attachments/assets/aed671c1-b7f7-4c9f-8553-485f8c0dd35e">

[start_up_timeline.json.zip](https://github.com/user-attachments/files/16515389/start_up_timeline.json.zip)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants