-
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
Parsing patterns when idling (performance follow-up for inserting patterns into containers) #29444
Changes from 8 commits
c0edc39
4298360
c1d85b4
b0b6b43
39bc7eb
34e4aad
ac25ca5
0e0e2b5
4df0a4e
8f07730
9a200cd
a8b3d46
b12d4d6
2945982
eb4e1e0
f402bd5
fb07d35
1a176d2
cac8f34
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 |
---|---|---|
@@ -1,3 +1,4 @@ | ||
export { default as transformStyles } from './transform-styles'; | ||
export * from './theme'; | ||
export * from './block-variation-transforms'; | ||
export { usePreParsePatterns as __experimentalPreParsePatterns } from './pre-parse-patterns'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { useSelect, select } from '@wordpress/data'; | ||
import { useEffect } from '@wordpress/element'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import { store as blockEditorStore } from '../store'; | ||
|
||
export function usePreParsePatterns() { | ||
const patterns = useSelect( | ||
( _select ) => | ||
_select( blockEditorStore ).getSettings() | ||
.__experimentalBlockPatterns, | ||
[] | ||
); | ||
|
||
useEffect( () => { | ||
if ( ! patterns?.length ) { | ||
return; | ||
} | ||
|
||
let handle; | ||
let index = -1; | ||
|
||
const callback = () => { | ||
index++; | ||
if ( index >= patterns.length ) { | ||
return; | ||
} | ||
|
||
const marker = `[usePreParsePatterns] process ${ index } ${ patterns[ index ]?.title }`; | ||
window.console.time( marker ); | ||
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.
|
||
select( blockEditorStore ).__experimentalGetParsedBlocks( | ||
youknowriad marked this conversation as resolved.
Show resolved
Hide resolved
|
||
patterns[ index ].content, | ||
index | ||
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. Is this second argument correct? 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. No, that's not needed anymore. Thanks! |
||
); | ||
window.console.timeEnd( marker ); | ||
|
||
handle = window.requestIdleCallback( callback ); | ||
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. we have the priority query package that seems perfect for these idle callback queues. Did you consider it? 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. Yes, I tried it. One problem I ran into is priority queue can't limit how many tasks to run per callback. It always tries to use up all the remaining processing time which didn't work quite well. It was noticeably laggier when opening up the editor because it tried to parse too many patterns. Since |
||
}; | ||
|
||
window.console.time( '[usePreParsePatterns] initiate' ); | ||
window.console.timeEnd( '[usePreParsePatterns] initiate' ); | ||
|
||
handle = window.requestIdleCallback( callback ); | ||
return () => window.cancelIdleCallback( handle ); | ||
}, [ patterns ] ); | ||
|
||
return null; | ||
} |
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.
For demonstration purposes only. I'll remove this once the PR is approved.
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.
Can we remove that code, in order to get accurate numbers in the job and judge whether the PR allows us to get the previous loading numbers?