Skip to content
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

perf(develop): skip query extraction if files are not dirty and schema didn't change #28595

Closed
wants to merge 1 commit into from

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Dec 13, 2020

Description

Every time we invoke query running machine we go through extracting queries step. This adds quite a bit of unnecesary work (specifically on larger sites). This is particularly felt in query on demand mode where we enter query running state in response to user actions in browser (running queries that are not up date on navigation and link hovers). In one of sites this adds ~1s delay in on demand query running that is quite unnecessary and degrade experience.

Unknowns:

  • as usual - investigate spots we clear schemaDirty and assess if this is safe (my gut feeling say that this is actually safe because schema rebuilding is governed by state machine, so there should be no "rogue" SET_SCHEMA events going on - but 🤷‍♂️ )

Related Issues

[ch19361]

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Dec 13, 2020
@KyleAMathews
Copy link
Contributor

🙌

@KyleAMathews
Copy link
Contributor

Do we track the dirtiness of individual page components? Not necessary for this PR but if we do (or could), that'd be another speedup — only extract from dirty page components.

@pieh pieh marked this pull request as ready for review December 14, 2020 08:53
@pieh pieh added topic: query on demand* and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Dec 14, 2020
@pieh
Copy link
Contributor Author

pieh commented Dec 14, 2020

Do we track the dirtiness of individual page components? Not necessary for this PR but if we do (or could), that'd be another speedup — only extract from dirty page components.

We don't track it on extraction level - we do track extracted and merged query texts (for query invalidation purposes). Keep in mind that queries append used fragments so it's not just page components (or files with static queries) alone that needs tracking. This is solvable of course but is touching different parts of code (those would be internals of query extraction and not top level orchestrating layer)

@pieh pieh marked this pull request as draft December 14, 2020 09:52
@pieh
Copy link
Contributor Author

pieh commented Dec 14, 2020

From chat with @ascorbic

@ascorbic:
You need to be super careful with that change. There are going to be places where we assume that jumping to that machine will re-extract queries
Don’t forget static images either. They’re generated during query extraction
Also check anything that jumps earlier than query-extraction too
@pieh:
I also do wonder - I use existing dirtyFiles thingie - but there are multiple handlers for it - so I do wonder if each consumer need separate dirty flag (i.e. webpack vs extraction)
I think this one definitely needs test cases added

@KyleAMathews
Copy link
Contributor

hmm ok given the above, it could just make sense to track the state of files in query extraction and skip files there that we know haven't changed.

@LekoArts LekoArts added topic: DX Developer Experience (e.g. Fast Refresh, i18n, SSR, page creation, starters) topic: core Relates to Gatsby's core (e.g. page loading, reporter, state machine) and removed topic: query on demand* labels May 28, 2021
@LekoArts LekoArts closed this Jul 20, 2022
@LekoArts LekoArts deleted the qod/skip-query-extraction-if-not-needed branch July 20, 2022 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: core Relates to Gatsby's core (e.g. page loading, reporter, state machine) topic: DX Developer Experience (e.g. Fast Refresh, i18n, SSR, page creation, starters)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants