Skip to content

Performance improvements to atrule() and trim() fns #55

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

Merged
merged 1 commit into from
Oct 23, 2013
Merged

Performance improvements to atrule() and trim() fns #55

merged 1 commit into from
Oct 23, 2013

Conversation

brettstimmerman
Copy link
Contributor

I found a couple tiny (code-wise) perf tweaks with a big impact today.

Before this change, atrule() was a cascade of potentially failing RegExps.
This change introduces a short-circuit that bypasses the RegExps if the
next character is not an '@'. make bench shows a 20-25% improvement in
op/s.

Also before this change, when the argument is falsy trim() calls replace()
on the empty string. This change guards against unnecessary calls to replace()
with a ternary. make bench shows a 2.5-5.5% improvement in op/s.

With both changes, make bench shows a combined 30-40% improvement.

Benchmarks were run 5 times and the results averaged.

@jonathanong
Copy link
Contributor

i would pull the atrule() one immediately.

however, the trim regexp is there for ie8 compatibility. we'd want to use trim or just inline it as a helper.

@brettstimmerman
Copy link
Contributor Author

Ah, good point about trim in IE8. Will fixup & re-push only the atrule() change.

@brettstimmerman
Copy link
Contributor Author

trim() can still be improved slightly with a guard against calling replace() on the empty string.

Before this change, `atrule()` was a cascade of potentially failing RegExps.
This change introduces a short-circuit that bypasses the RegExps if the
next character is not an '@'. `make bench` shows a 20-25% improvement in
op/s.

Also before this change, when the argument is falsy `trim()` calls `replace()`
on the empty string. This change guards against unnecessary calls to `replace()`
with a ternary. `make bench` shows a 2.5-5.5% improvement in op/s.

With both changes, make bench shows a combined 30-40% improvement.

Benchmarks were run 5 times and the results averaged.
@tj
Copy link
Member

tj commented Oct 23, 2013

cool thanks!

tj added a commit that referenced this pull request Oct 23, 2013
Performance improvements to atrule() and trim() fns
@tj tj merged commit 4368352 into reworkcss:master Oct 23, 2013
@brettstimmerman brettstimmerman deleted the chore/perf-improvements branch October 23, 2013 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants