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

Add hasInserterItems selector and improve getInserterItems selector performance #11845

Merged
merged 1 commit into from Nov 15, 2018

Conversation

Projects
None yet
4 participants
@youknowriad
Contributor

youknowriad commented Nov 14, 2018

The getInserterItems is called often (even if the inserter in not open) and its memoization is not performant.

In this PR, I'm adding a new hasInserterItems selector which is more performant as it doesn't have to create inserter items and can return quickly after one insertable item is found.

I also refactored a little bit the selectors to avoid using a memoized version of canInsertBlockType as I think it's just not efficient to call memoized selectors inside other memoized selectors.

I don't have the numbers as it's very hard to measure but I'd love your thoughts. I'm also considering dropping the memoization of these two selectors entirely because the cache is invalidated too often IMO.

@youknowriad youknowriad self-assigned this Nov 14, 2018

@youknowriad youknowriad requested review from noisysocks and WordPress/gutenberg-core Nov 14, 2018

@tofumatt

Explanation makes sense to me and I think the code reads fine. 👍

Show resolved Hide resolved packages/editor/src/store/selectors.js
@aduth

This comment has been minimized.

Member

aduth commented Nov 14, 2018

Anecdotally, I observed in a recent performance audit that this function (getInserterItems) accounted for 20% of total JavaScript time spent while actively typing within a Columns block content:

image

(Which is to say: This pull request should hopefully help)

@youknowriad youknowriad force-pushed the try/improve-inserter-performance branch from 726d03a to a4137cc Nov 15, 2018

@youknowriad youknowriad merged commit 4a54cc1 into master Nov 15, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@youknowriad youknowriad deleted the try/improve-inserter-performance branch Nov 15, 2018

@lkraav

This comment has been minimized.

lkraav commented Nov 15, 2018

I built and ran this minute's master 4a54cc1 on our sample problematic content, and was not able to detect any user-visible block editing performance improvement yet. Quietly assuming here it requires more pieces to fall in place.

grey-rsi pushed a commit to OnTheGoSystems/gutenberg that referenced this pull request Nov 22, 2018

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