-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ const results = { | |
firstContentfulPaint: [], | ||
firstBlock: [], | ||
type: [], | ||
typeWithoutInspector: [], | ||
typeContainer: [], | ||
focus: [], | ||
listViewOpen: [], | ||
|
@@ -91,6 +92,40 @@ test.describe( 'Post Editor Performance', () => { | |
} | ||
} ); | ||
|
||
async function type( target, metrics, key ) { | ||
// The first character typed triggers a longer time (isTyping change). | ||
// It can impact the stability of the metric, so we exclude it. It | ||
// probably deserves a dedicated metric itself, though. | ||
const samples = 10; | ||
const throwaway = 1; | ||
const iterations = samples + throwaway; | ||
|
||
// Start tracing. | ||
await metrics.startTracing(); | ||
|
||
// Type the testing sequence into the empty paragraph. | ||
await target.type( 'x'.repeat( iterations ), { | ||
delay: BROWSER_IDLE_WAIT, | ||
// The extended timeout is needed because the typing is very slow | ||
// and the `delay` value itself does not extend it. | ||
timeout: iterations * BROWSER_IDLE_WAIT * 2, // 2x the total time to be safe. | ||
} ); | ||
|
||
// Stop tracing. | ||
await metrics.stopTracing(); | ||
|
||
// Get the durations. | ||
const [ keyDownEvents, keyPressEvents, keyUpEvents ] = | ||
metrics.getTypingEventDurations(); | ||
|
||
// Save the results. | ||
for ( let i = throwaway; i < iterations; i++ ) { | ||
results[ key ].push( | ||
keyDownEvents[ i ] + keyPressEvents[ i ] + keyUpEvents[ i ] | ||
); | ||
} | ||
} | ||
|
||
test.describe( 'Typing', () => { | ||
let draftId = null; | ||
|
||
|
@@ -110,37 +145,34 @@ test.describe( 'Post Editor Performance', () => { | |
name: /Empty block/i, | ||
} ); | ||
|
||
// The first character typed triggers a longer time (isTyping change). | ||
// It can impact the stability of the metric, so we exclude it. It | ||
// probably deserves a dedicated metric itself, though. | ||
const samples = 10; | ||
const throwaway = 1; | ||
const iterations = samples + throwaway; | ||
await type( paragraph, metrics, 'type' ); | ||
} ); | ||
} ); | ||
|
||
// Start tracing. | ||
await metrics.startTracing(); | ||
test.describe( 'Typing (without inspector)', () => { | ||
let draftId = null; | ||
|
||
// Type the testing sequence into the empty paragraph. | ||
await paragraph.type( 'x'.repeat( iterations ), { | ||
delay: BROWSER_IDLE_WAIT, | ||
// The extended timeout is needed because the typing is very slow | ||
// and the `delay` value itself does not extend it. | ||
timeout: iterations * BROWSER_IDLE_WAIT * 2, // 2x the total time to be safe. | ||
} ); | ||
test( 'Setup the test post', async ( { admin, perfUtils, editor } ) => { | ||
await admin.createNewPost(); | ||
await perfUtils.loadBlocksForLargePost(); | ||
await editor.insertBlock( { name: 'core/paragraph' } ); | ||
draftId = await perfUtils.saveDraft(); | ||
} ); | ||
|
||
// Stop tracing. | ||
await metrics.stopTracing(); | ||
test( 'Run the test', async ( { admin, perfUtils, metrics, page } ) => { | ||
await admin.editPost( draftId ); | ||
await perfUtils.disableAutosave(); | ||
const toggleButton = page | ||
.getByRole( 'region', { name: 'Editor settings' } ) | ||
.getByRole( 'button', { name: 'Close Settings' } ); | ||
await toggleButton.click(); | ||
const canvas = await perfUtils.getCanvas(); | ||
|
||
// Get the durations. | ||
const [ keyDownEvents, keyPressEvents, keyUpEvents ] = | ||
metrics.getTypingEventDurations(); | ||
const paragraph = canvas.getByRole( 'document', { | ||
name: /Empty block/i, | ||
} ); | ||
|
||
// Save the results. | ||
for ( let i = throwaway; i < iterations; i++ ) { | ||
results.type.push( | ||
keyDownEvents[ i ] + keyPressEvents[ i ] + keyUpEvents[ i ] | ||
); | ||
} | ||
await type( paragraph, metrics, 'typeWithoutInspector' ); | ||
} ); | ||
} ); | ||
|
||
|
@@ -166,37 +198,7 @@ test.describe( 'Post Editor Performance', () => { | |
.first(); | ||
await firstParagraph.click(); | ||
|
||
// The first character typed triggers a longer time (isTyping change). | ||
// It can impact the stability of the metric, so we exclude it. It | ||
// probably deserves a dedicated metric itself, though. | ||
const samples = 10; | ||
const throwaway = 1; | ||
const iterations = samples + throwaway; | ||
|
||
// Start tracing. | ||
await metrics.startTracing(); | ||
|
||
// Start typing in the middle of the text. | ||
await firstParagraph.type( 'x'.repeat( iterations ), { | ||
delay: BROWSER_IDLE_WAIT, | ||
// The extended timeout is needed because the typing is very slow | ||
// and the `delay` value itself does not extend it. | ||
timeout: iterations * BROWSER_IDLE_WAIT * 2, // 2x the total time to be safe. | ||
} ); | ||
|
||
// Stop tracing. | ||
await metrics.stopTracing(); | ||
|
||
// Get the durations. | ||
const [ keyDownEvents, keyPressEvents, keyUpEvents ] = | ||
metrics.getTypingEventDurations(); | ||
|
||
// Save the results. | ||
for ( let i = throwaway; i < iterations; i++ ) { | ||
results.typeContainer.push( | ||
keyDownEvents[ i ] + keyPressEvents[ i ] + keyUpEvents[ i ] | ||
); | ||
} | ||
await type( firstParagraph, metrics, 'typeContainer' ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. |
||
} ); | ||
} ); | ||
|
||
|
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