SomMeri / less4j

Less language is an extension of css and less4j compiles it into regular css. Less adds several dynamic features into css: variables, expressions, nested rules, and so on. Less was designed to be compatible with css and any correct css file is also correct less file.
145 stars 47 forks source link

implemented several performance improvements #320

Closed a701440 closed 8 years ago

a701440 commented 8 years ago

This branch incorporates several performance improvements. I filed some of the as separate issues recently. In our large projects these changes provide approximately 3x-4x speed-up.

Included here:

  1. Externalizing AST cache, so that it can be reused across compile calls
  2. Copy on write functionality for LocalScope to avoid unnecessary clone() calls
  3. Caching of CssClass names so that it's not recomputed needlessly in comparators
  4. ListsComparator optimization to increase speed and decrease garbage creation
  5. ExtendsSolver optimization to not create ArrayLists when not needed
  6. ScopesTreeView does not create HashMap any more
  7. Added source jar generation in POM.xml. Makes it easier for me to setup ivy dependencies in my project.
SomMeri commented 8 years ago

Thank you a lot, this is great.

On plans: I am trying to fix import reference in less.js which should take few more days, hopefully less then two weeks. I do not want to interrupt it, but rather finish it at once (interruptions costed me too much on this task) I will merge this in after that and make a new release.

If you need new release of less4j sooner, let me know.

a701440 commented 8 years ago

One more item that is fairly inefficient is creation of ExpressionEvaluator instances. For example I see that it's created many times in ReferenceSolver.solveCalls. Every creation appears to take the same scope parameter that is not changing in the method. Can the ExpressionEvaluator be created only once in that method ?

SomMeri commented 8 years ago

@a701440 If the scope parameter is guaranteed not to change, then it should work. The scope is the only difference between expression evaluator instances.

Looking at it, the List<FunctionsPackage> functions could be created only once for all expression evaluators, it is the same every time. Not sure if it would make things measurably faster, but the possibility is there.

a701440 commented 8 years ago

I have moved the creation ExpressionEvaluator out of the loop and it helped. At this point I don't see any low hanging fruit in the profiles I have captured. Most of the time is now spent in various comparators mathching selectors, etc. I guess this can be improved with more efficient data structures (indexes or hash maps), but it would require a more invasive surgery. I think LocalScope should be insulated better, so that Copy on Write would be less fragile. As it stands it would be very easy to cause unintended and unnecessary "clone" calls by calling getLocalVariables() even without any "write" intent.

a701440 commented 8 years ago

Hello. Are there any plans for a release in the next few days? We are about to deploy our custom build, but if an 'official' build is coming we would wait a bit.

SomMeri commented 8 years ago

Probably depends on how you define "few days". I am about to start merging your pull request and can release after that. Release itself is not much work if there is no documentation to be done.

I want to have a look at a place you called "fragile" closer and at a cache api, so I may release by the end of this week and definitely will release by the end of next week.

SomMeri commented 8 years ago

Less4j 1.15.3 was just released. Thank you for profiling it and pull request :).