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

Performance tuning (#1615) #1625

Merged
merged 5 commits into from
Dec 16, 2013
Merged

Performance tuning (#1615) #1625

merged 5 commits into from
Dec 16, 2013

Conversation

fredburger
Copy link

Performance tuning (#1615)

Various performance optimizations, without refactoring or messing up too much (hopefully?).

33% - 50% reduction in compilation time for bootstrap.less.

Compilation times for bootstrap.less:

Env. Mode 1.5.0-master 1.5.0-perf-tune % reduction
Node lessc (single run) 719 ms 484 ms -33%
Node JIT'ed (8 runs) 509 ms 228 ms -55%
FF Browser 2198 ms 832 ms -62%
Chrome Browser 1092 ms 797 ms -27%
IE9 Browser 1720 ms 965 ms -44%

@lukeapage
Copy link
Member

wow, some good work, I will spend some time looking through this and then merge. I had a quick glance through and didn't see anything scary.

@fredburger
Copy link
Author

Cool, thx!

@oyejorge
Copy link
Contributor

oyejorge commented Dec 6, 2013

Yes!
I'm in the process of updating my less.php compiler to 1.5 and so far these changes are proving to be just as effective in php.

}
rulesetNode.joinSelectors(paths, context, rulesetNode.selectors);
if (!selectors) { rulesetNode.rules = null; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note to self, check this works with & {}

@peol
Copy link

peol commented Dec 9, 2013

I just tried this branch in a rather complex app I have (>350 less files with varying includes) and it processed them ~45% faster(!), 👍 to get this in.

@lukeapage
Copy link
Member

I'm in the process of doing it. I may revert some of the micro
optimizations if they make the code harder to understand but the bigger
things that make most of the difference will stay.

@peol
Copy link

peol commented Dec 9, 2013

That's awesome. Let me know if you want me to try anything out.

Cheers

Sorry for being brief, I sent this from my phone.

/Andrée

On 9 dec 2013, at 21:35, Luke Page [email protected] wrote:

I'm in the process of doing it. I may revert some of the micro
optimizations if they make the code harder to understand but the bigger
things that make most of the difference will stay.


Reply to this email directly or view it on
GitHubhttps://github.com//pull/1625#issuecomment-30170711
.

@lukeapage lukeapage merged commit 56c3338 into less:master Dec 16, 2013
@lukeapage
Copy link
Member

thanks, this is committed.

I made a few rollbacks, mainly around the jshint boss option (now on) e.g.

var a;
if (a = blah()) {
}

and "micro" caching. I don't think they made a difference, I still see a good improvment in speed, so thanks.

@bedney
Copy link

bedney commented Dec 17, 2013

Thank you fredburger for authoring these significant performance optimizations and to lukeapage for integrating them!

Great work!!

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.

5 participants