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(gatsby-plugin-mdx): Stop clobbering the same file over and over again #27974

Merged
merged 4 commits into from Jan 5, 2021

Conversation

pvdz
Copy link
Contributor

@pvdz pvdz commented Nov 11, 2020

This function (cacheScope) takes a bunch of imports and exports, generates a snippet of JS, conditionally runs it through Babel, and writes the resulting snippet out to a file. The filename is a hash based on the resulting data.

This means that if cacheScope receives the same inputs, it will have the same behavior. Writing the same output to the same file, over and over again. This PR stops that.

It uses a global set to use as session cache which I think is safe since these files should not be deleted during a build (not even develop).

Tested this on gabe-fs-mdx at 100k pages.

The sourcing step went down from 260s to 200s, roughly a 25% perf win.

(Unfortunately the page query step is still 50 minutes)

This work is roughly based on #25808, which I abandoned after some time (I forgot why). I feel pretty confident about this PR.

@pvdz pvdz requested a review from a team as a code owner November 11, 2020 15:36
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Nov 11, 2020
@pvdz pvdz added topic: MDX topic: performance Related to runtime & build performance and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Nov 11, 2020
@wardpeet
Copy link
Contributor

How does memory usage look like? We probably keep many strings in memory, what does gatsbyjs.com say?

@pvdz
Copy link
Contributor Author

pvdz commented Nov 12, 2020

No idea. Will have to test this later. Currently have everything wired up for tracing.

KyleAMathews
KyleAMathews previously approved these changes Jan 5, 2021
Copy link
Contributor

@KyleAMathews KyleAMathews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@gatsbot
Copy link

gatsbot bot commented Jan 5, 2021

Danger run resulted in 1 fail and 1 markdown; to find out more, see the checks page.

Generated by 🚫 dangerJS

@KyleAMathews KyleAMathews added this to To cherry-pick in V2 Release hotfixes via automation Jan 5, 2021
@KyleAMathews KyleAMathews merged commit 3d100e3 into master Jan 5, 2021
@KyleAMathews KyleAMathews deleted the stopclobbering branch January 5, 2021 16:44
@vladar vladar removed this from To cherry-pick in V2 Release hotfixes Jan 5, 2021
@vladar vladar added this to To cherry-pick in Release candidate via automation Jan 5, 2021
vladar pushed a commit that referenced this pull request Jan 5, 2021
…again (#27974)

* perf(gatsby-plugin-mdx): Stop clobbering the same file over and over again

* Undo the await part, can be separate PR

Co-authored-by: gatsbybot <mathews.kyle+gatsbybot@gmail.com>
(cherry picked from commit 3d100e3)
@vladar vladar moved this from To cherry-pick to Backport PR opened in Release candidate Jan 5, 2021
gatsbybot pushed a commit that referenced this pull request Jan 5, 2021
…again (#27974) (#28874)

* perf(gatsby-plugin-mdx): Stop clobbering the same file over and over again

* Undo the await part, can be separate PR

Co-authored-by: gatsbybot <mathews.kyle+gatsbybot@gmail.com>
(cherry picked from commit 3d100e3)

Co-authored-by: Peter van der Zee <209817+pvdz@users.noreply.github.com>
@vladar vladar moved this from Backport PR opened to Backported in Release candidate Jan 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: performance Related to runtime & build performance
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants