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

Preserve object identity when loading config, for improved future caching. #6326

Merged
merged 6 commits into from
Sep 29, 2017

Conversation

loganfsmyth
Copy link
Member

Q                       A
Fixed Issues N
Patch: Bug Fix? N
Major: Breaking Change? N, I think
Minor: New Feature?
Tests Added/Pass?
Spec Compliancy?
License MIT
Doc PR
Any Dependency Changes?

This change does essentially nothing on its own, but it opens up a bunch of options for us in the future. By preserving the object identity of as much of our config as we can, we can better cache processing around those configs.

In the future

The simplest case for this is that with this we can validate a config file and cache it by just doing

const isValid = makeWeakCache(config => {
  return doExpensiveValidation(config);
});

and the doExpensiveValidation will only be executed once for a given config object. Any time Babel runs on the same .babelrc, it'll never have to run that validation again.

We'll also be able to apply that caching for plugins and presets, so if you're babelrc is

{
    presets: ['es2015']
}

currently in Babel 6, the preset function for es2015 will run every time Babel runs on a file. Not only that, but Babel has to figure out where the es2015 package is in the filesystem. That's a ton of extra work for absolutely no reason. If we know the config hasn't changed, we can skip calling it again. The only downside is that if a preset uses globals somehow for config, we could cache in a bad state. Given this, we can re-use the same caching API I added for .babelrc.js files in presets too.

And we have the opposite problem in plugins currently. Plugins are always cached for performance, but this means we're locked with the plugin API as function (babelAPI){ while presets get additional data as function (babelAPI, options, {dirname}). By allowing caching by object identity, we can remove the permacache restriction from plugins, allowing they to accept options as top-level parameters, just like presets do.

I have this caching all done on a branch, and it improves the time to go from the name of the file to compile, to a fully verified config object and an array of plugins by ~2-3x, while allowing plugins to have access to options up front, which is a huge win in my mind.

In this PR

The most complex part of this PR is that I've implemented identity preservation rules for arguments passed to Babel too. Babel's top-level options object passed in programmatically is something that is commonly either mutated often, or passed as a fresh object. This means it's essentially impossible for us to do the caching I mentioned above for plugins/presets/whatever.

One approach would be to freeze every options object passed to Babel, so we could know the user hasn't gone and mutated it between calls, but the DX on that would be a pain for everyone.

To try to strike a middle ground, in this PR I've approached the issue by caching individual properties of the programmatic options object by their identity instead. So for opts.plugins, we generate a new cacheable config object for just the plugins. The same is true for .env and .presets, since these are all objects that have a unique identity.

This means that babel won't be able to cache the option validation I mentioned above for the programmatic options, but it will be able to do it for the plugins, presets, and anything nested under the env block.

I think it's a solid compromise, but I'm curious for feedback.

@babel-bot
Copy link
Collaborator

babel-bot commented Sep 27, 2017

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/5069/

@hzoo
Copy link
Member

hzoo commented Sep 29, 2017

won't be able to cache the option validation I mentioned above for the programmatic options

Was it caching before though?

@hzoo hzoo closed this Sep 29, 2017
@hzoo hzoo reopened this Sep 29, 2017
@loganfsmyth
Copy link
Member Author

loganfsmyth commented Sep 29, 2017

@hzoo No it wasn't cached before, it's more that we will cache plugins and presets (once I add caching), so something like

var opts = {
  presets: ['es2015'],
};

babel.transform("", opts);

opts.presets.push('react');

babel.transform("", opts);

would not use react on the second call, because the presets object is exactly the same array.

I think that's fine as an edge case though.

If they did

opts.presets = opts.presets.concat('react');

though, so it was a whole new array, it'd work.

@hzoo
Copy link
Member

hzoo commented Sep 29, 2017

would that be an issue during testing if someone might code like that ^? Probably the only case I can think of, unless someone does 2 separate transforms for some reason

@loganfsmyth
Copy link
Member Author

I don't think it's an edge case worth worrying about honestly. If a transform isn't being run, people will notice and fix it easily enough by copying the array.

@existentialism existentialism added the PR: Internal 🏠 A type of pull request used for our changelog categories label Sep 29, 2017
@loganfsmyth loganfsmyth merged commit 828aec7 into babel:master Sep 29, 2017
@loganfsmyth loganfsmyth deleted the preserve-config-identity branch September 29, 2017 22:36
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 5, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Internal 🏠 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants