Skip to content

Commit

Permalink
Improve importantify() performance (#201)
Browse files Browse the repository at this point in the history
* Avoid unused capture group in IMPORTANT_RE

I was rolling through the code here looking for some opportunities to
speed it up and I noticed that there is a capture group in this regex
that isn't being used. By changing `(` to `(?:` we can avoid capturing
this group while still being able to use the parens for grouping.

* Use string for importantify replace

I was curious about the performance characteristics of various options
here, and using a string in this way seems to have the edge, at least in
Firefox (35% faster) and Safari (12% faster)--Chrome performance seems
pretty similar for all possibilities. I also find this version more
readable, so it seems like we may as well go with this option.

https://jsperf.com/replace-string-vs-function/

* Simplify importantify regex

I don't think we actually need a capture group at all here and can
accomplish the same thing by just anchoring on the end of the string.
This reduces the runtime of this function in my benchmark from ~105ms to
~35ms.

* Scope importantify to just the value

Since we only call this function from one place, and we already have the
CSS property name and value split out, we can further optimize this hot
path by only running importantify on the CSS property value. In my
benchmark, this change speeds up this map by ~22% (~128ms to ~100ms).

* Remove regex from importantify

Now that this has been sufficiently simplified, we can easily remove the
regex entirely.

* Add bracket optimization to importantify

In our microbenchmark, adding this optimization performs a good amount
better. Since it is a simple optimization to add, it seems like we may
as well do it.

https://jsperf.com/slice-vs-bracket-string-check-with-imporant

I considered using bracket notation for the whole check, but I decided
that was too verbose. So instead, I decided to just check for one
character and follow up with a more thorough but concise check
afterward. This optimization favors the case where the style does not
already have `!important` on it, which is by and large likely to be the
general case.

* Move conditional out of loop in generateCSSRuleset

This cleans up the code a bit, and is also faster!

https://jsperf.com/if-vs-extra-function/1
  • Loading branch information
lencioni authored and xymostech committed Mar 7, 2017
1 parent 5e9a0cb commit dbb3879
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 13 deletions.
12 changes: 7 additions & 5 deletions src/generate.js
Original file line number Diff line number Diff line change
Expand Up @@ -284,11 +284,13 @@ export const generateCSSRuleset = (
})
);

const rules = prefixedRules.map(([key, value]) => {
const stringValue = stringifyValue(key, value);
const ret = `${kebabifyStyleName(key)}:${stringValue};`;
return useImportant === false ? ret : importantify(ret);
}).join("");
const transformValue = (useImportant === false)
? stringifyValue
: (key, value) => importantify(stringifyValue(key, value));

const rules = prefixedRules
.map(([key, value]) => `${kebabifyStyleName(key)}:${transformValue(key, value)};`)
.join("");

if (rules) {
return `${selector}{${rules}}`;
Expand Down
20 changes: 12 additions & 8 deletions src/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,11 +215,15 @@ function murmurhash2_32_gc(str) {
export const hashObject = (object /* : ObjectMap */) /* : string */ => murmurhash2_32_gc(JSON.stringify(object));


const IMPORTANT_RE = /^([^:]+:.*?)( !important)?;$/;

// Given a single style rule string like "a: b;", adds !important to generate
// "a: b !important;".
export const importantify = (string /* : string */) /* : string */ =>
string.replace(
IMPORTANT_RE,
(_, base) => base + " !important;");
// Given a single style value string like the "b" from "a: b;", adds !important
// to generate "b !important".
export const importantify = (string /* : string */) /* : string */ => (
// Bracket string character access is very fast, and in the default case we
// normally don't expect there to be "!important" at the end of the string
// so we can use this simple check to take an optimized path. If there
// happens to be a "!" in this position, we follow up with a more thorough
// check.
(string[string.length - 10] === '!' && string.slice(-11) === ' !important')
? string
: `${string} !important`
);

0 comments on commit dbb3879

Please sign in to comment.