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

Conversation

@iskakaushik
Copy link
Contributor

This reverts commit 04afdad.

This had to be reverted because of flutter/flutter#76216. This has been fixed in 28603f1.

Copy link
Member

@chinmaygarde chinmaygarde left a comment

Choose a reason for hiding this comment

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

I specifically reviewed 28603f1. I believe the rest was identical to the reverted patch.

@iskakaushik iskakaushik merged commit 874b8e8 into flutter:master Mar 11, 2021
@iskakaushik iskakaushik deleted the reland-pipeline branch March 11, 2021 18:56
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 11, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 11, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 12, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 12, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 12, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 12, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 12, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 12, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 12, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 12, 2021
@chinmaygarde
Copy link
Member

@iskakaushik This removed the PipelineItem flow that DevTools was using to detect the duration of a frame as it was being serviced on the two threads. I think we can recreate the flow in this new mechanism. Can we revert it (sadly, once again) and add that flow to unblock DevTools or do you want to fix forward by adding the flow?

cc @kenzieschmoll

@iskakaushik
Copy link
Contributor Author

Yup, will revert it. I want to debug flutter/flutter#78257 (comment) as well so maybe two birds with one stone. Though this particular stone (reverting this change) has been used multiple times.

iskakaushik added a commit to iskakaushik/engine that referenced this pull request Mar 16, 2021
iskakaushik added a commit to iskakaushik/engine that referenced this pull request Mar 16, 2021
@chinmaygarde
Copy link
Member

Right, subtle changes to scheduling are no fun to push through. We need to tackle flutter/flutter#27609 correctly anyway. Right now, I doubt we have the bandwidth to create a test harness that allows us to reason about frame scheduling and trace emission.

@flar
Copy link
Contributor

flar commented Mar 17, 2021

Are they prepared for flows that start and never end? Should we establish a "flow aborted" event that they can look for to see when frames are dropped on the floor?

@chinmaygarde
Copy link
Member

Are they prepared for flows that start and never end?

Likely not. The current behavior is to "end" flows that are being prematurely terminated. That is, there is no difference between a successful or unsuccessful flow. But it always ends.

@flar
Copy link
Contributor

flar commented Mar 18, 2021

It looks like there already were existing ways to prematurely end flows before so this should be no different - but it will be more common.

We might want to ask them if they'd like a way to distinguish dropped frames from completed frames now that they will be more common.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants