Skip to content

Conversation

@pieh
Copy link
Contributor

@pieh pieh commented Dec 23, 2020

On certain site:

Current:

success Building development bundle - 162.795s
success Building development bundle - 171.725s
success Building development bundle - 171.985s
success Building development bundle - 162.793s
success Building development bundle - 183.253s

With this change:

success Building development bundle - 113.453s
success Building development bundle - 110.612s
success Building development bundle - 112.168s
success Building development bundle - 112.159s
success Building development bundle - 110.889s

Just the packages/gatsby/src/utils/babel-loader-helpers.js change (biggest gain of all the caching added in this PR - at least for site I was benchmarking)

success Building development bundle - 118.803s
success Building development bundle - 123.173s
success Building development bundle - 119.359s
success Building development bundle - 120.030s
success Building development bundle - 118.143s

There might be more things to cache around this, but this was likely the most heavy thing that will yield biggest benefit

Explanation for changes:

babelLoader.custom(callback) runs callback function for every input file. babel.createConfigItem seems to instantiate new plugin/preset every time it's called. This seems to run https://github.com/babel/babel/blob/5b5b548036cde05b424d481c65c1c4a27c6eabc6/packages/babel-preset-env/src/normalize-options.js#L202-L263 separately for every input file even tho our options for @babel/preset-env don't change for given stage (@babel/preset-env is just example - it's similar for every babel plugin/preset, just this one seemed to be heaviest on site I was profiling).

Because babel@7 support nested .babelrc we can't not use babelLoader.custom (that would be breaking change/regression), but we can cache/memoize things we know don't change and avoid doing repetitive work.

The tricky part would be to memoize the partial config merging internals -

const mergeConfigItemOptions = ({ items, itemToMerge, type, babel }) => {
const index = _.findIndex(
items,
i => i.file.resolved === itemToMerge.file.resolved
)
// If this exist, merge the options, otherwise, add it to the array
if (index !== -1) {
items[index] = babel.createConfigItem(
[
itemToMerge.file.resolved,
_.merge({}, items[index].options, itemToMerge.options),
],
{
type,
}
)
} else {
items.push(itemToMerge)
}
return items
}

So instead I opted to memoize one level higher -

// If there is no filesystem babel config present, add our fallback
// presets/plugins.
if (!partialConfig.hasFilesystemConfig()) {
options = {
...options,
plugins: requiredPlugins,
presets: [...fallbackPresets, ...requiredPresets],
}
} else {
// With a babelrc present, only add our required plugins/presets
options = {
...options,
plugins: [...options.plugins, ...requiredPlugins],
presets: [...options.presets, ...requiredPresets],
}
}
// Merge in presets/plugins added from gatsby plugins.
reduxPresets.forEach(preset => {
options.presets = mergeConfigItemOptions({
items: options.presets,
itemToMerge: preset,
type: `preset`,
babel,
})
})
reduxPlugins.forEach(plugin => {
options.plugins = mergeConfigItemOptions({
items: options.plugins,
itemToMerge: plugin,
type: `plugin`,
babel,
})
})

Rationale behind this is that number of .babelrc files wouldn't be very high and even if we do some repetitive merging for each .babelrc file - at least we won't be doing this for every input file. On the site I was profiling there actually wasn't any extra .babelrc files and memoizing on that level (on top of memoizing prepareOptions) yielded extra 5s-10s gains.

There is always opportunity to figure out better memoizing setup to support cases of multiple .babelrc files better - but it's not clear to me how common using multiple of those configs is (maybe we should track this with telemetry and count "cache misses" per stage)

[ch27747]

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Dec 23, 2020
@pieh pieh added topic: webpack/babel Webpack or babel and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Dec 23, 2020
@KyleAMathews
Copy link
Contributor

This PR is similar #27974

@wardpeet wardpeet self-assigned this Jan 11, 2021
@pieh pieh force-pushed the perf/cache-babel-config-items branch from 0428d03 to 39efdf3 Compare April 1, 2021 10:54
@pieh pieh force-pushed the perf/cache-babel-config-items branch from 39efdf3 to cede740 Compare April 3, 2021 10:29
@pieh pieh marked this pull request as ready for review April 3, 2021 13:28
@pieh pieh changed the base branch from master to e2e/babelrc April 6, 2021 08:57
Base automatically changed from e2e/babelrc to master April 6, 2021 11:53
@pieh pieh force-pushed the perf/cache-babel-config-items branch from eebbbfa to f0bfcd7 Compare April 6, 2021 12:00
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.

Looks great! Like using webpack as a file watcher. Huge perf improvements 🙌

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

Labels

topic: webpack/babel Webpack or babel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants