Skip to content

Performance tool: Warn when interrupting an in-progress tree#11480

Merged
acdlite merged 2 commits intofacebook:masterfrom
acdlite:warnoninterruptions
Nov 7, 2017
Merged

Performance tool: Warn when interrupting an in-progress tree#11480
acdlite merged 2 commits intofacebook:masterfrom
acdlite:warnoninterruptions

Conversation

@acdlite
Copy link
Copy Markdown
Collaborator

@acdlite acdlite commented Nov 7, 2017

Adds a warning to the performance tool when an in-progress update is interrupted by equal or higher priority work.

warning = 'Interrupted an in-progress update';
} else if (commitCountInCurrentWorkLoop > 1) {
warning = 'There were cascading updates';
}
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@gaearon Wasn't sure what to do here. Should we show both warnings? How should they be formatted?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Maybe show more important one? There's not much screen space there.

@acdlite acdlite force-pushed the warnoninterruptions branch from cc5ea67 to c65a6a0 Compare November 7, 2017 13:37
@gaearon
Copy link
Copy Markdown
Collaborator

gaearon commented Nov 7, 2017

Is interrupting always bad?

@acdlite
Copy link
Copy Markdown
Collaborator Author

acdlite commented Nov 7, 2017

Not necessarily, though neither are cascading updates. Maybe we should add a different type of label formatted for non-scary information?

@acdlite
Copy link
Copy Markdown
Collaborator Author

acdlite commented Nov 7, 2017

An example of something that's always bad is expiration.


export function startWorkLoopTimer(): void {
export function startWorkLoopTimer(nextUnitOfWork: Fiber | null): void {
currentFiber = nextUnitOfWork;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This should be inside the if (enableUserTimingAPI) check.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Updated

@acdlite acdlite force-pushed the warnoninterruptions branch from bdbe678 to 30b5334 Compare November 7, 2017 16:25
Copy link
Copy Markdown
Collaborator

@gaearon gaearon left a comment

Choose a reason for hiding this comment

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

LG

@acdlite acdlite merged commit 05f3ecc into facebook:master Nov 7, 2017
@acdlite acdlite deleted the warnoninterruptions branch November 7, 2017 16:52
Ethan-Arrowood pushed a commit to Ethan-Arrowood/react that referenced this pull request Dec 8, 2017
…k#11480)

* Performance tool: Warn when interrupting an in-progress tree

* Include the name of the component that caused the interruption
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants