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

Fix slow autoprefixer calls in *-no-vendor-prefix #4971

Closed
victor-homyakov opened this issue Oct 7, 2020 · 7 comments · Fixed by #5312
Closed

Fix slow autoprefixer calls in *-no-vendor-prefix #4971

victor-homyakov opened this issue Oct 7, 2020 · 7 comments · Fixed by #5312
Labels
status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule

Comments

@victor-homyakov
Copy link
Contributor

What is the problem you're trying to solve?

Stylelint is often launched as a pre-commit hook. It is important in this case, how fast the tool starts. If it takes several seconds to start (no matter how many files are committed), developers may notice that commits are slower than usual, and prefer some other tool, if it exists.

The overall performance of stylelint is influenced by a few heavy dependencies, that consume significant time when stylelint starts:

  1. require('@stylelint/postcss-css-in-js') in https://github.com/stylelint/stylelint/blob/master/lib/syntaxes/syntax-css-in-js.js#L3 may take about 900 ms on my notebook (of course, exact value depends on hardware characteristics and many other factors). In other words, when stylelint decides to use this syntax, 900 ms is added to execution time.
  2. When linting with *-no-vendor-prefix rules (at-rule-no-vendor-prefix, media-feature-name-no-vendor-prefix, property-no-vendor-prefix, selector-no-vendor-prefix, value-no-vendor-prefix), require('autoprefixer') takes about 500 ms here: https://github.com/stylelint/stylelint/blob/master/lib/utils/isAutoprefixable.js#L3.
  3. require('@stylelint/postcss-markdown') in syntax-markdown - about 200 ms
  4. require('postcss-less') in syntax-html, syntax-less, syntax-markdown - about 200 ms

In the worst case, the pre-commit hook may take about two seconds to lint even a one-line change in style.

What solution would you like to see?

In some cases, it is possible to require only used properties from dependency. I.e. instead of

const autoprefixer = require('autoprefixer');

const prefixes = new Prefixes(
	autoprefixer.data.prefixes,
	new Browsers(autoprefixer.data.browsers, []),
);

one could write

const data = {
	browsers: require('caniuse-lite/dist/unpacker/agents').agents,
	prefixes: require('autoprefixer/data/prefixes')
};

const prefixes = new Prefixes(
	data.prefixes,
	new Browsers(data.browsers, []),
);

This code is faster (about 400 ms instead of 500) because it does not evaluate the whole autoprefixer code, but it is more complex. Probably import-lazy also could be used more widely. Third-party libraries like autoprefixer and babel-core could be improved in order to provide more granular (== faster) imports.

I understand that there is no common solution, only partial mitigation. But performance tests indicate that developers of stylelint think about its speed, and right now slow startup adds the same or even more to the whole time than the rules themselves.

What do you think?

@alexander-akait
Copy link
Member

I think it is good idea to load syntaxes lazy 👍

@victor-homyakov
Copy link
Contributor Author

Syntaxes should be already lazy loaded: https://github.com/stylelint/stylelint/blob/master/lib/syntaxes/index.js

@hudochenkov
Copy link
Member

I think it's a duplicate for #2454

In some cases, it is possible to require only used properties from dependency.

Relying on dependencies internals, which are not public API of a library, could break any time. Even with patch dependency release.

@ybiquitous
Copy link
Member

How about the following lazy load? Wrapping access to autoprefixer and prefixes with the functions.

--- a/lib/utils/isAutoprefixable.js
+++ b/lib/utils/isAutoprefixable.js
@@ -1,9 +1,5 @@
 'use strict';
 
-const autoprefixer = require('autoprefixer');
-const Browsers = require('autoprefixer/lib/browsers');
-const Prefixes = require('autoprefixer/lib/prefixes');
-
 /**
  * Use Autoprefixer's secret powers to determine whether or
  * not a certain CSS identifier contains a vendor prefix that
@@ -13,10 +9,31 @@ const Prefixes = require('autoprefixer/lib/prefixes');
  * vendor prefixes.
  */
 
-const prefixes = new Prefixes(
-	autoprefixer.data.prefixes,
-	new Browsers(autoprefixer.data.browsers, []),
-);
+// NOTE: Lazy loading to avoid the performance degrade due to the heavy packages.
+
+/** @type {import('autoprefixer') | undefined} */
+let autoprefixerCache;
+let autoprefixer = () => {
+	if (!autoprefixerCache) {
+		autoprefixerCache = require('autoprefixer');
+	}
+
+	return autoprefixerCache;
+};
+
+/** @type {import('autoprefixer/lib/prefixes') | undefined} */
+let prefixesCache;
+const prefixes = () => {
+	if (!prefixesCache) {
+		const Browsers = require('autoprefixer/lib/browsers');
+		const Prefixes = require('autoprefixer/lib/prefixes');
+		const data = autoprefixer().data;
+
+		prefixesCache = new Prefixes(data.prefixes, new Browsers(data.browsers, []));
+	}
+
+	return prefixesCache;
+};
 
 /**
  * Most identifier types have to be looked up in a unique way,
@@ -28,7 +45,7 @@ module.exports = {
 	 * @returns {boolean}
 	 */
 	atRuleName(identifier) {
-		return Boolean(prefixes.remove[`@${identifier.toLowerCase()}`]);
+		return Boolean(prefixes().remove[`@${identifier.toLowerCase()}`]);
 	},
 
 	/**
@@ -36,7 +53,7 @@ module.exports = {
 	 * @returns {boolean}
 	 */
 	selector(identifier) {
-		return prefixes.remove.selectors.some((/** @type {{ prefixed: string}} */ selectorObj) => {
+		return prefixes().remove.selectors.some((/** @type {{ prefixed: string}} */ selectorObj) => {
 			return identifier.toLowerCase() === selectorObj.prefixed;
 		});
 	},
@@ -54,7 +71,7 @@ module.exports = {
 	 * @returns {boolean}
 	 */
 	property(identifier) {
-		return Boolean(autoprefixer.data.prefixes[prefixes.unprefixed(identifier.toLowerCase())]);
+		return Boolean(autoprefixer().data.prefixes[prefixes().unprefixed(identifier.toLowerCase())]);
 	},
 
 	/**
@@ -64,8 +81,8 @@ module.exports = {
 	 * @returns {boolean}
 	 */
 	propertyValue(prop, value) {
-		const possiblePrefixableValues =
-			(prefixes.remove[prop.toLowerCase()] && prefixes.remove[prop.toLowerCase()].values) || false;
+		const remove = prefixes().remove[prop.toLowerCase()];
+		const possiblePrefixableValues = (remove && remove.values) || false;
 
 		return (
 			possiblePrefixableValues &&

@victor-homyakov
Copy link
Contributor Author

In some cases, it is possible to require only used properties from dependency.

Relying on dependencies internals, which are not public API of a library, could break any time. Even with patch dependency release.

Of course, but open-source libraries like autoprefixer and babel-core also could be improved in order to provide more granular dependencies

@jeddy3
Copy link
Member

jeddy3 commented Nov 1, 2020

@victor-homyakov Thank you for digging into the performance and for the write-up. Trying to make stylelint faster has been an ongoing endevour, but there's still lots more that can be done!

How about the following lazy load? Wrapping access to autoprefixer and prefixes with the functions.

@ybiquitous LGTM. Do you want to rustle up a pull request (that will close this issue)?

As @hudochenkov said, this is a duplicate of #2454 and we should continue the conversation over there.

Of course, but open-source libraries like autoprefixer and babel-core also could be improved in order to provide more granular dependencies

Yes, I agree. Hopefully, @ybiquitous suggestion will help with autoprefixer, otherwise we can suggest improvements upstream.

As you pointed out, the syntaxes are responsible for a lot of the slowness. I feel like we may need to revisit how syntaxes work in general, especially how packages like postcss-markdown and postcss-syntax require nested syntaxes like postcss-less. We can pick this up in #2454.

@jeddy3 jeddy3 changed the title CLI startup performance Refactor to cache autoprefixer calls Nov 1, 2020
@jeddy3 jeddy3 added status: ready to implement is ready to be worked on by someone type: refactor an improvement to the code structure labels Nov 1, 2020
@jeddy3 jeddy3 changed the title Refactor to cache autoprefixer calls Fix slow autoprefixer calls in *-no-vendor-prefix Nov 1, 2020
@jeddy3 jeddy3 added type: bug a problem with a feature or rule and removed type: refactor an improvement to the code structure labels Nov 1, 2020
@ybiquitous
Copy link
Member

@ybiquitous LGTM. Do you want to rustle up a pull request (that will close this issue)?

I've opened PR #5019 based on my comment: #4971 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule
5 participants