-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Performance: measure typing without inspector #56753
Conversation
Size Change: 0 B Total Size: 1.72 MB ℹ️ View Unchanged
|
@@ -48,6 +49,9 @@ export interface WPPerformanceResults { | |||
type?: number; | |||
minType?: number; | |||
maxType?: number; | |||
typeWithoutInspector?: number; | |||
minTypeWithoutInspector?: number; | |||
maxTypeWithoutInspector?: number; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd remove the min/max to be honest, not that useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, can remove it
keyDownEvents[ i ] + keyPressEvents[ i ] + keyUpEvents[ i ] | ||
); | ||
} | ||
await type( firstParagraph, metrics, 'typeContainer' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I intentionally kept the way we compute "type in container" different than the regular "type".
One types and waits, the other doesn't wait. My thinking was that both of these metrics are useful but they are different. This PR reverts that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading the diff, maybe this has been reverted elsewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok never mind, it was reverted before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This had an impact on some numbers like "type in container", I guess this changed a bit how we compute the metric. Do you know what changed exactly? |
Also, why do you think this PR seem to have impacted "block select" metric as well? |
cc @WunderBart might also be interested in this PR |
@youknowriad, if closing the Editor Settings panel is persistent for the current editor instance, it will likely affect all the subsequent tests. It should be reopened after the test to keep the other numbers stable. edit: Nevermind, just noticed #56780 |
What?
Adds a typing perf metric with the inspector panel closed.
Why?
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast